[mutter/wip/carlosg/xwayland-startup-side-channel: 65/69] xwayland: Do not block on Xwayland initialization

[Carlos Garnacho <carlosg src gnome org>]

commit aa84b6b566374b3c6e252f42dd61c02b66104968
Author: Carlos Garnacho <carlosg gnome org>
Date:   Mon Nov 18 14:04:24 2019 +0100

    xwayland: Do not block on Xwayland initialization
    
    We artificially made Xwayland initialization synchronous, as we used
    to rely on MetaX11Display and other bits during meta_display_open().
    With support for Xwayland on demand and --no-x11, this is certainly
    not the case.
    
    So drop the main loop surrounding Xwayland initialization, and turn
    it into an async operation called from meta_display_init_x11(). This
    function is turned then into the high-level entry point that will
    get you from no X server to having a MetaX11Display.
    
    The role of meta_init() in Xwayland initialization is thus reduced
    to setting up the sockets. Notably no processes are spawned from here,
    deferring that till there is a MetaDisplay to poke.
    
    https://gitlab.gnome.org/GNOME/mutter/merge_requests/944

 src/core/display.c                  | 42 ++++++++++++++++-
 src/wayland/meta-wayland-private.h  |  1 -
 src/wayland/meta-xwayland-private.h |  8 ++++
 src/wayland/meta-xwayland.c         | 89 ++++++++++++++++++-------------------
 4 files changed, 92 insertions(+), 48 deletions(-)
---
diff --git a/src/core/display.c b/src/core/display.c
index 8cbde3faf..649dff1f5 100644
--- a/src/core/display.c
+++ b/src/core/display.c
@@ -714,6 +714,25 @@ meta_display_init_x11_finish (MetaDisplay   *display,
   return TRUE;
 }
 
+#ifdef HAVE_WAYLAND
+static void
+on_xserver_started (MetaXWaylandManager *manager,
+                    GAsyncResult        *result,
+                    gpointer             user_data)
+{
+  GTask *task = user_data;
+  GError *error = NULL;
+  gboolean retval;
+
+  retval = meta_xwayland_start_xserver_finish (manager, result, &error);
+
+  if (error)
+    g_task_return_error (task, error);
+  else
+    g_task_return_boolean (task, retval);
+}
+#endif
+
 void
 meta_display_init_x11 (MetaDisplay         *display,
                        GCancellable        *cancellable,
@@ -725,7 +744,22 @@ meta_display_init_x11 (MetaDisplay         *display,
   task = g_task_new (display, cancellable, callback, user_data);
   g_task_set_source_tag (task, meta_display_init_x11);
 
-  g_task_return_boolean (task, TRUE);
+#ifdef HAVE_WAYLAND
+  if (meta_is_wayland_compositor ())
+    {
+      MetaWaylandCompositor *compositor = meta_wayland_compositor_get_default ();
+
+      meta_xwayland_start_xserver (&compositor->xwayland_manager,
+                                   cancellable,
+                                   (GAsyncReadyCallback) on_xserver_started,
+                                   g_object_ref (task));
+    }
+  else
+#endif
+    {
+      g_task_return_boolean (task, TRUE);
+    }
+
   g_object_unref (task);
 }
 
@@ -855,7 +889,11 @@ meta_display_open (void)
   else
     {
       if (meta_get_x11_display_policy () == META_DISPLAY_POLICY_MANDATORY)
-        meta_display_init_x11 (display, NULL, on_x11_initialized, NULL);
+        {
+          meta_display_init_x11 (display, NULL,
+                                 (GAsyncReadyCallback) on_x11_initialized,
+                                 NULL);
+        }
       timestamp = meta_display_get_current_time_roundtrip (display);
     }
 
diff --git a/src/wayland/meta-wayland-private.h b/src/wayland/meta-wayland-private.h
index 9b99674e0..aa8e4deab 100644
--- a/src/wayland/meta-wayland-private.h
+++ b/src/wayland/meta-wayland-private.h
@@ -57,7 +57,6 @@ typedef struct
 
   GCancellable *xserver_died_cancellable;
   GSubprocess *proc;
-  GMainLoop *init_loop;
 
   GList *x11_windows;
 
diff --git a/src/wayland/meta-xwayland-private.h b/src/wayland/meta-xwayland-private.h
index 75cd04715..51dfa633b 100644
--- a/src/wayland/meta-xwayland-private.h
+++ b/src/wayland/meta-xwayland-private.h
@@ -42,4 +42,12 @@ gboolean meta_xwayland_dnd_handle_event (XEvent *xevent);
 
 const MetaWaylandDragDestFuncs * meta_xwayland_selection_get_drag_dest_funcs (void);
 
+void meta_xwayland_start_xserver (MetaXWaylandManager *manager,
+                                  GCancellable        *cancellable,
+                                  GAsyncReadyCallback  callback,
+                                  gpointer             user_data);
+gboolean meta_xwayland_start_xserver_finish (MetaXWaylandManager  *manager,
+                                             GAsyncResult         *result,
+                                             GError              **error);
+
 #endif /* META_XWAYLAND_PRIVATE_H */
diff --git a/src/wayland/meta-xwayland.c b/src/wayland/meta-xwayland.c
index eec403493..ae84e0672 100644
--- a/src/wayland/meta-xwayland.c
+++ b/src/wayland/meta-xwayland.c
@@ -582,17 +582,6 @@ add_local_user_to_xhost (Display *xdisplay)
   XAddHost (xdisplay, &host_entry);
 }
 
-static void
-xserver_finished_init (MetaXWaylandManager *manager)
-{
-  /* At this point xwayland is all setup to start accepting
-   * connections so we can quit the transient initialization mainloop
-   * and unblock meta_wayland_init() to continue initializing mutter.
-   * */
-  g_main_loop_quit (manager->init_loop);
-  g_clear_pointer (&manager->init_loop, g_main_loop_unref);
-}
-
 static void
 on_init_x11_cb (MetaDisplay  *display,
                 GAsyncResult *result,
@@ -609,45 +598,53 @@ on_displayfd_ready (int          fd,
                     GIOCondition condition,
                     gpointer     user_data)
 {
-  MetaXWaylandManager *manager = user_data;
-  MetaDisplay *display = meta_get_display ();
+  GTask *task = user_data;
 
   /* The server writes its display name to the displayfd
    * socket when it's ready. We don't care about the data
    * in the socket, just that it wrote something, since
    * that means it's ready. */
-  xserver_finished_init (manager);
-
-  if (meta_get_x11_display_policy () == META_DISPLAY_POLICY_ON_DEMAND)
-    {
-      meta_display_init_x11 (display, NULL,
-                             (GAsyncReadyCallback) on_init_x11_cb, NULL);
-    }
+  g_task_return_boolean (task, TRUE);
+  g_object_unref (task);
 
   return G_SOURCE_REMOVE;
 }
 
-static gboolean
-meta_xwayland_start_xserver (MetaXWaylandManager *manager)
+void
+meta_xwayland_start_xserver (MetaXWaylandManager *manager,
+                             GCancellable        *cancellable,
+                             GAsyncReadyCallback  callback,
+                             gpointer             user_data)
 {
   int xwayland_client_fd[2];
   int displayfd[2];
   g_autoptr(GSubprocessLauncher) launcher = NULL;
   GSubprocessFlags flags;
   GError *error = NULL;
+  g_autoptr (GTask) task = NULL;
+
+  task = g_task_new (NULL, cancellable, callback, user_data);
+  g_task_set_source_tag (task, meta_xwayland_start_xserver);
+  g_task_set_task_data (task, manager, NULL);
 
   /* We want xwayland to be a wayland client so we make a socketpair to setup a
    * wayland protocol connection. */
   if (socketpair (AF_UNIX, SOCK_STREAM | SOCK_CLOEXEC, 0, xwayland_client_fd) < 0)
     {
-      g_warning ("xwayland_client_fd socketpair failed\n");
-      return FALSE;
+      g_task_return_new_error (task,
+                               G_IO_ERROR,
+                               g_io_error_from_errno (errno),
+                               "xwayland_client_fd socketpair failed");
+      return;
     }
 
   if (socketpair (AF_UNIX, SOCK_STREAM | SOCK_CLOEXEC, 0, displayfd) < 0)
     {
-      g_warning ("displayfd socketpair failed\n");
-      return FALSE;
+      g_task_return_new_error (task,
+                               G_IO_ERROR,
+                               g_io_error_from_errno (errno),
+                               "displayfd socketpair failed");
+      return;
     }
 
   /* xwayland, please. */
@@ -681,24 +678,28 @@ meta_xwayland_start_xserver (MetaXWaylandManager *manager)
                                                NULL);
   if (!manager->proc)
     {
-      g_error ("Failed to spawn Xwayland: %s", error->message);
-      return FALSE;
+      g_task_return_error (task, error);
+      return;
     }
 
   manager->xserver_died_cancellable = g_cancellable_new ();
   g_subprocess_wait_async (manager->proc, manager->xserver_died_cancellable,
                            xserver_died, NULL);
-  g_unix_fd_add (displayfd[0], G_IO_IN, on_displayfd_ready, manager);
+  g_unix_fd_add (displayfd[0], G_IO_IN, on_displayfd_ready,
+                 g_steal_pointer (&task));
   manager->client = wl_client_create (manager->wayland_display,
                                       xwayland_client_fd[0]);
+}
 
-  /* We need to run a mainloop until we know xwayland has a binding
-   * for our xserver interface at which point we can assume it's
-   * ready to start accepting connections. */
-  manager->init_loop = g_main_loop_new (NULL, FALSE);
-  g_main_loop_run (manager->init_loop);
+gboolean
+meta_xwayland_start_xserver_finish (MetaXWaylandManager  *manager,
+                                    GAsyncResult         *result,
+                                    GError              **error)
+{
+  g_assert (g_task_get_source_tag (G_TASK (result)) ==
+            meta_xwayland_start_xserver);
 
-  return TRUE;
+  return g_task_propagate_boolean (G_TASK (result), error);
 }
 
 static gboolean
@@ -706,10 +707,13 @@ xdisplay_connection_activity_cb (gint         fd,
                                  GIOCondition cond,
                                  gpointer     user_data)
 {
-  MetaXWaylandManager *manager = user_data;
+  if (meta_get_x11_display_policy () == META_DISPLAY_POLICY_ON_DEMAND)
+    {
+      MetaDisplay *display = meta_get_display ();
 
-  if (!meta_xwayland_start_xserver (manager))
-    g_critical ("Could not start Xserver");
+      meta_display_init_x11 (display, NULL,
+                             (GAsyncReadyCallback) on_init_x11_cb, NULL);
+    }
 
   return G_SOURCE_REMOVE;
 }
@@ -792,18 +796,13 @@ meta_xwayland_init (MetaXWaylandManager *manager,
   manager->wayland_display = wl_display;
   policy = meta_get_x11_display_policy ();
 
-  if (policy == META_DISPLAY_POLICY_MANDATORY)
-    {
-      return meta_xwayland_start_xserver (manager);
-    }
-  else if (policy == META_DISPLAY_POLICY_ON_DEMAND)
+  if (policy == META_DISPLAY_POLICY_ON_DEMAND)
     {
       g_unix_fd_add (manager->abstract_fd, G_IO_IN,
                      xdisplay_connection_activity_cb, manager);
-      return TRUE;
     }
 
-  return FALSE;
+  return TRUE;
 }
 
 static void