gl/wayland: move roundtrip on show to window thread

This makes it thread safe and fixes a possible deadlock.

Keeping the roundtrip off the window thread will result in two different
threads call wl_display_dispatch_queue() for the same queue which
violates the assumption for _dispatch_queue()'s thread-safety
guarantees.

https://bugzilla.gnome.org/show_bug.cgi?id=788754
https://bugzilla.gnome.org/show_bug.cgi?id=792156
https://bugzilla.gnome.org/show_bug.cgi?id=758984

diff --git a/gst-libs/gst/gl/wayland/gstglwindow_wayland_egl.c b/gst-libs/gst/gl/wayland/gstglwindow_wayland_egl.c
index 4c71bf5..3852818 100644 (file)
--- a/gst-libs/gst/gl/wayland/gstglwindow_wayland_egl.c
+++ b/gst-libs/gst/gl/wayland/gstglwindow_wayland_egl.c
@@ -292,8 +292,8 @@ create_surfaces (GstGLWindowWaylandEGL * window_egl)
           wl_shell_get_shell_surface (display->shell,
           window_egl->window.surface);
       if (window_egl->window.queue)
-        wl_proxy_set_queue ((struct wl_proxy *) window_egl->window.
-            shell_surface, window_egl->window.queue);
+        wl_proxy_set_queue ((struct wl_proxy *) window_egl->
+            window.shell_surface, window_egl->window.queue);
 
       wl_shell_surface_add_listener (window_egl->window.shell_surface,
           &shell_surface_listener, window_egl);
@@ -450,7 +450,7 @@ gst_gl_window_wayland_egl_set_window_handle (GstGLWindow * window,
 }
 
 static void
-gst_gl_window_wayland_egl_show (GstGLWindow * window)
+_roundtrip_async (GstGLWindow * window)
 {
   GstGLDisplayWayland *display_wayland =
       GST_GL_DISPLAY_WAYLAND (window->display);
@@ -464,6 +464,16 @@ gst_gl_window_wayland_egl_show (GstGLWindow * window)
 }
 
 static void
+gst_gl_window_wayland_egl_show (GstGLWindow * window)
+{
+  GstGLWindowWaylandEGL *window_egl = GST_GL_WINDOW_WAYLAND_EGL (window);
+
+  create_surfaces (window_egl);
+
+  gst_gl_window_send_message (window, (GstGLWindowCB) _roundtrip_async, window);
+}
+
+static void
 window_resize (GstGLWindowWaylandEGL * window_egl, guint width, guint height)
 {
   GstGLWindow *window = GST_GL_WINDOW (window_egl);

diff --git a/gst-libs/gst/gl/wayland/wayland_event_source.c b/gst-libs/gst/gl/wayland/wayland_event_source.c
index ffb1503..12a0668 100644 (file)
--- a/gst-libs/gst/gl/wayland/wayland_event_source.c
+++ b/gst-libs/gst/gl/wayland/wayland_event_source.c
@@ -35,6 +35,7 @@
 #include <stdint.h>
 #include <stdlib.h>
 #include <wayland-client.h>
+#include <gst/gst.h>
 
 #include "wayland_event_source.h"
 
@@ -43,6 +44,8 @@ sync_callback (void *data, struct wl_callback *callback, uint32_t serial)
 {
   gboolean *done = data;
 
+  GST_TRACE ("roundtrip done. callback:%p", callback);
+
   *done = TRUE;
   wl_callback_destroy (callback);
 }
@@ -51,7 +54,14 @@ static const struct wl_callback_listener sync_listener = {
   sync_callback
 };
 
-/* only thread safe iff called on the same thread @queue is being dispatched on */
+/* only thread safe iff called on the same thread @queue is being dispatched on.
+ * Otherwise, two prepare_read{_queue}()'s can be indicated for the same
+ * queue and dispatch{_queue}() may be called for different threads which
+ * will cause deadlocks as no guarantees for thread-safety are given when
+ * pumping the same queue from multiple threads.
+ * As a concrete example, if the wayland event source (below) for a @queue is
+ * running on a certain thread, then this function must only be called in that
+ * thread (with that @queue). */
 gint
 gst_gl_wl_display_roundtrip_queue (struct wl_display *display,
     struct wl_event_queue *queue)
@@ -60,30 +70,38 @@ gst_gl_wl_display_roundtrip_queue (struct wl_display *display,
   gboolean done = FALSE;
   gint ret = 0;
 
+  GST_TRACE ("roundtrip start for dpy %p and queue %p", display, queue);
+
   if (queue) {
     /* creating a wl_proxy and setting the queue is racy with the dispatching
      * of the default queue */
     while (wl_display_prepare_read_queue (display, queue) != 0) {
-      if ((ret = wl_display_dispatch_queue_pending (display, queue)) < 0)
+      if ((ret = wl_display_dispatch_queue_pending (display, queue)) < 0) {
         return ret;
+      }
     }
   }
   if (!(callback = wl_display_sync (display))) {
     return -1;
   }
+  GST_TRACE ("create roundtrip callback %p", callback);
   wl_callback_add_listener (callback, &sync_listener, &done);
   if (queue) {
     wl_proxy_set_queue ((struct wl_proxy *) callback, queue);
     wl_display_cancel_read (display);
-    while (!done && ret >= 0)
+    while (!done && ret >= 0) {
       ret = wl_display_dispatch_queue (display, queue);
+    }
   } else {
-    while (!done && ret >= 0)
+    while (!done && ret >= 0) {
       ret = wl_display_dispatch (display);
+    }
   }
 
   if (ret == -1 && !done)
     wl_callback_destroy (callback);
+  GST_TRACE ("roundtrip done for dpy %p and queue %p. ret %i", display, queue,
+      ret);
 
   return ret;
 }
@@ -156,10 +174,11 @@ wayland_event_source_dispatch (GSource * base,
 {
   WaylandEventSource *source = (WaylandEventSource *) base;
 
-  if (source->queue)
+  if (source->queue) {
     wl_display_dispatch_queue_pending (source->display, source->queue);
-  else
+  } else {
     wl_display_dispatch_pending (source->display);
+  }
   source->pfd.revents = 0;
 
   if (callback)