[Spice-devel] [PATCH 20/22] Move the X11CursorUpdater and X11CursorThread classes in a separate file

Christophe de Dinechin christophe at dinechin.org
Wed Feb 28 15:43:23 UTC 2018


From: Christophe de Dinechin <dinechin at redhat.com>

This makes it possible to remove X11 dependencies from the main agent file.

Note: it may be unsafe to call XCloseDisplay from the destructor.
Doing some experiments throwing exceptions, I noticed messages such as:

[xcb] Unknown request in queue while dequeuing
[xcb] Most likely this is a multi-threaded client and XInitThreads
      has not been called
[xcb] Aborting, sorry about that.
spice-streaming-agent: xcb_io.c:165: dequeue_pending_request:
Assertion `!xcb_xlib_unknown_req_in_deq' failed.
Aborted (core dumped)

The stack trace looks like this:

    #1  0x00007ffff65cf4da in abort () from /lib64/libc.so.6
    #2  0x00007ffff65c5d67 in __assert_fail_base () from /lib64/libc.so.6
    #3  0x00007ffff65c5e12 in __assert_fail () from /lib64/libc.so.6
    #4  0x00007ffff76b6ecc in dequeue_pending_request () from /lib64/libX11.so.6
    #5  0x00007ffff76b7d20 in _XReply () from /lib64/libX11.so.6
    #6  0x00007ffff76b36cd in XSync () from /lib64/libX11.so.6
    #7  0x00007ffff769494e in XCloseDisplay () from /lib64/libX11.so.6

If we hit this problem in practice, we may need to remove the
XCloseDisplay call from the destructor.

Signed-off-by: Christophe de Dinechin <dinechin at redhat.com>
---
 src/Makefile.am               |   2 +
 src/message.hpp               |   2 +
 src/spice-streaming-agent.cpp | 121 +-----------------------------------------
 src/x11-cursor.cpp            |  65 +++++++++++++++++++++++
 src/x11-cursor.hpp            |  91 +++++++++++++++++++++++++++++++
 5 files changed, 161 insertions(+), 120 deletions(-)
 create mode 100644 src/x11-cursor.cpp
 create mode 100644 src/x11-cursor.hpp

diff --git a/src/Makefile.am b/src/Makefile.am
index 923a103..4a03e5e 100644
--- a/src/Makefile.am
+++ b/src/Makefile.am
@@ -58,4 +58,6 @@ spice_streaming_agent_SOURCES = \
 	stream.cpp \
 	stream.hpp \
 	errors.cpp \
+	x11-cursor.hpp \
+	x11-cursor.cpp \
 	$(NULL)
diff --git a/src/message.hpp b/src/message.hpp
index 28b3e28..650bd66 100644
--- a/src/message.hpp
+++ b/src/message.hpp
@@ -6,6 +6,8 @@
 #ifndef SPICE_STREAMING_AGENT_MESSAGE_HPP
 #define SPICE_STREAMING_AGENT_MESSAGE_HPP
 
+#include "stream.hpp"
+
 #include <spice/stream-device.h>
 
 namespace spice
diff --git a/src/spice-streaming-agent.cpp b/src/spice-streaming-agent.cpp
index c401a34..2264af2 100644
--- a/src/spice-streaming-agent.cpp
+++ b/src/spice-streaming-agent.cpp
@@ -7,6 +7,7 @@
 #include "concrete-agent.hpp"
 #include "stream.hpp"
 #include "message.hpp"
+#include "x11-cursor.hpp"
 #include "hexdump.h"
 #include "mjpeg-fallback.hpp"
 
@@ -36,8 +37,6 @@
 #include <vector>
 #include <string>
 #include <functional>
-#include <X11/Xlib.h>
-#include <X11/extensions/Xfixes.h>
 
 using namespace spice::streaming_agent;
 
@@ -86,63 +85,6 @@ public:
     }
 };
 
-class X11CursorMessage : public Message<StreamMsgCursorSet, X11CursorMessage, STREAM_TYPE_CURSOR_SET>
-{
-public:
-    X11CursorMessage(XFixesCursorImage *cursor): Message(cursor) {}
-    static size_t size(XFixesCursorImage *cursor)
-    {
-        return sizeof(payload_t) + sizeof(uint32_t) * pixel_count(cursor);
-    }
-
-    void write_message_body(Stream &stream, XFixesCursorImage *cursor)
-    {
-        StreamMsgCursorSet msg = {
-            .width = cursor->width,
-            .height = cursor->height,
-            .hot_spot_x = cursor->xhot,
-            .hot_spot_y = cursor->yhot,
-            .type = SPICE_CURSOR_TYPE_ALPHA,
-            .padding1 = { },
-            .data = { }
-        };
-
-        size_t pixcount = pixel_count(cursor);
-        size_t pixsize = pixcount * sizeof(uint32_t);
-        std::unique_ptr<uint32_t[]> pixels(new uint32_t[pixcount]);
-        uint32_t *pixbuf = pixels.get();
-        fill_pixels(cursor, pixcount, pixbuf);
-
-        stream.write_all("cursor message", &msg, sizeof(msg));
-        stream.write_all("cursor pixels", pixbuf, pixsize);
-    }
-
-private:
-    static size_t pixel_count(XFixesCursorImage *cursor)
-    {
-        return cursor->width * cursor->height;
-    }
-
-    void fill_pixels(XFixesCursorImage *cursor, unsigned count, uint32_t *pixbuf)
-    {
-        for (unsigned i = 0; i < count; ++i) {
-            pixbuf[i] = cursor->pixels[i];
-        }
-    }
-};
-
-class X11CursorUpdater
-{
-public:
-    X11CursorUpdater(Stream &stream);
-    ~X11CursorUpdater();
-    void send_cursor_changes();
-
-private:
-    Stream &stream;
-    Display *display;
-};
-
 class FrameLog
 {
 public:
@@ -182,67 +124,6 @@ void FrameLog::dump(const void *buffer, size_t length)
     }
 }
 
-class X11CursorThread
-{
-public:
-    X11CursorThread(Stream &stream);
-    static void record_cursor_changes(X11CursorThread *self) { self->updater.send_cursor_changes(); }
-
-private:
-    X11CursorUpdater updater;
-    std::thread thread;
-};
-
-X11CursorUpdater::X11CursorUpdater(Stream &stream)
-    : stream(stream),
-      display(XOpenDisplay(NULL))
-{
-    if (display == NULL) {
-        throw Error("failed to open display").syslog();
-    }
-}
-
-X11CursorUpdater::~X11CursorUpdater()
-{
-    XCloseDisplay(display);
-}
-
-void X11CursorUpdater::send_cursor_changes()
-{
-    unsigned long last_serial = 0;
-
-    int event_base, error_base;
-    if (!XFixesQueryExtension(display, &event_base, &error_base)) {
-        syslog(LOG_WARNING, "XFixesQueryExtension failed, not sending cursor changes\n");
-        return; // also terminates the X11CursorThread if that's how we were launched
-    }
-    Window rootwindow = DefaultRootWindow(display);
-    XFixesSelectCursorInput(display, rootwindow, XFixesDisplayCursorNotifyMask);
-
-    while (true) {
-        XEvent event;
-        XNextEvent(display, &event);
-        if (event.type != event_base + 1) {
-            continue;
-        }
-
-        XFixesCursorImage *cursor = XFixesGetCursorImage(display);
-        if (!cursor || cursor->cursor_serial == last_serial) {
-            continue;
-        }
-
-        last_serial = cursor->cursor_serial;
-        stream.send<X11CursorMessage>(cursor);
-    }
-}
-
-X11CursorThread::X11CursorThread(Stream &stream)
-    : updater(stream),
-      thread(record_cursor_changes, this)
-{
-    thread.detach();
-}
-
 }} // namespace spice::streaming_agent
 
 bool quit_requested = false;
diff --git a/src/x11-cursor.cpp b/src/x11-cursor.cpp
new file mode 100644
index 0000000..6abc258
--- /dev/null
+++ b/src/x11-cursor.cpp
@@ -0,0 +1,65 @@
+/* X11 cursor transmission
+ *
+ * \copyright
+ * Copyright 2018 Red Hat Inc. All rights reserved.
+ */
+#include "x11-cursor.hpp"
+
+#include <syslog.h>
+
+namespace spice
+{
+namespace streaming_agent
+{
+
+X11CursorUpdater::X11CursorUpdater(Stream &stream)
+    : stream(stream),
+      display(XOpenDisplay(NULL))
+{
+    if (display == NULL) {
+        throw Error("failed to open display").syslog();
+    }
+}
+
+X11CursorUpdater::~X11CursorUpdater()
+{
+    XCloseDisplay(display);
+}
+
+void X11CursorUpdater::send_cursor_changes()
+{
+    unsigned long last_serial = 0;
+
+    int event_base, error_base;
+    if (!XFixesQueryExtension(display, &event_base, &error_base)) {
+        syslog(LOG_WARNING, "XFixesQueryExtension failed, not sending cursor changes\n");
+        return; // also terminates the X11CursorThread if that's how we were launched
+    }
+    Window rootwindow = DefaultRootWindow(display);
+    XFixesSelectCursorInput(display, rootwindow, XFixesDisplayCursorNotifyMask);
+
+    while (true) {
+        XEvent event;
+        XNextEvent(display, &event);
+        if (event.type != event_base + 1) {
+            continue;
+        }
+
+        XFixesCursorImage *cursor = XFixesGetCursorImage(display);
+        if (!cursor || cursor->cursor_serial == last_serial) {
+            continue;
+        }
+
+        last_serial = cursor->cursor_serial;
+        stream.send<X11CursorMessage>(cursor);
+    }
+}
+
+X11CursorThread::X11CursorThread(Stream &stream)
+    : updater(stream),
+      thread(record_cursor_changes, this)
+{
+    thread.detach();
+}
+
+}} // namespace spic::streaming_agent
diff --git a/src/x11-cursor.hpp b/src/x11-cursor.hpp
new file mode 100644
index 0000000..2038b1d
--- /dev/null
+++ b/src/x11-cursor.hpp
@@ -0,0 +1,91 @@
+/* X11 cursor transmission
+ *
+ * \copyright
+ * Copyright 2018 Red Hat Inc. All rights reserved.
+ */
+#ifndef SPICE_STREAMING_AGENT_X11_CURSOR_HPP
+#define SPICE_STREAMING_AGENT_X11_CURSOR_HPP
+
+#include "message.hpp"
+
+#include <spice-streaming-agent/errors.hpp>
+
+#include <thread>
+#include <X11/Xlib.h>
+#include <X11/extensions/Xfixes.h>
+
+namespace spice {
+namespace streaming_agent {
+
+class X11CursorMessage : public Message<StreamMsgCursorSet, X11CursorMessage,
+                                        STREAM_TYPE_CURSOR_SET>
+{
+public:
+    X11CursorMessage(XFixesCursorImage *cursor): Message(cursor) {}
+    static size_t size(XFixesCursorImage *cursor)
+    {
+        return sizeof(payload_t) + sizeof(uint32_t) * pixel_count(cursor);
+    }
+
+    void write_message_body(Stream &stream, XFixesCursorImage *cursor)
+    {
+        StreamMsgCursorSet msg = {
+            .width = cursor->width,
+            .height = cursor->height,
+            .hot_spot_x = cursor->xhot,
+            .hot_spot_y = cursor->yhot,
+            .type = SPICE_CURSOR_TYPE_ALPHA,
+            .padding1 = { },
+            .data = { }
+        };
+
+        size_t pixcount = pixel_count(cursor);
+        size_t pixsize = pixcount * sizeof(uint32_t);
+        std::unique_ptr<uint32_t[]> pixels(new uint32_t[pixcount]);
+        uint32_t *pixbuf = pixels.get();
+        fill_pixels(cursor, pixcount, pixbuf);
+
+        stream.write_all("cursor message", &msg, sizeof(msg));
+        stream.write_all("cursor pixels", pixbuf, pixsize);
+    }
+
+private:
+    static size_t pixel_count(XFixesCursorImage *cursor)
+    {
+        return cursor->width * cursor->height;
+    }
+
+    void fill_pixels(XFixesCursorImage *cursor, unsigned count, uint32_t *pixbuf)
+    {
+        for (unsigned i = 0; i < count; ++i) {
+            pixbuf[i] = cursor->pixels[i];
+        }
+    }
+};
+
+class X11CursorUpdater
+{
+public:
+    X11CursorUpdater(Stream &stream);
+    ~X11CursorUpdater();
+    void send_cursor_changes();
+
+private:
+    Stream &stream;
+    Display *display;
+};
+
+class X11CursorThread
+{
+public:
+    X11CursorThread(Stream &stream);
+    static void record_cursor_changes(X11CursorThread *self) { self->updater.send_cursor_changes(); }
+
+private:
+    X11CursorUpdater updater;
+    std::thread thread;
+};
+
+}} // namespace spice::streaming_agent
+
+#endif // SPICE_STREAMING_AGENT_X11_CURSOR_HPP
-- 
2.13.5 (Apple Git-94)



More information about the Spice-devel mailing list