[glib/benzea/systemd-transient-scope: 1/2] gdesktopappinfo: Handle task completion from spawn function




commit 06e4354ba3d50f3a0c91af39ff2871d1ce2f5038
Author: Benjamin Berg <bberg redhat com>
Date:   Tue Jul 28 12:11:13 2020 +0200

    gdesktopappinfo: Handle task completion from spawn function
    
    This allows delaying the return of the task until all dbus calls (in
    particular the ones to setup the scope) have finished.
    
    This fixes the behaviour of the previous commit which would not
    correctly move the process into the scope if the application exited
    right after the task returned.

 gio/gdesktopappinfo.c | 214 ++++++++++++++++++++++++++++++++++----------------
 1 file changed, 147 insertions(+), 67 deletions(-)
---
diff --git a/gio/gdesktopappinfo.c b/gio/gdesktopappinfo.c
index 7320a66eb..fec334f97 100644
--- a/gio/gdesktopappinfo.c
+++ b/gio/gdesktopappinfo.c
@@ -2812,11 +2812,17 @@ create_systemd_scope (GDBusConnection    *session_bus,
   g_free (unit_name);
 }
 
+typedef struct {
+  GTask *task;
+  int fd;
+} ScopeCreatedData;
+
 static void
 systemd_scope_created_cb (GObject      *object,
                           GAsyncResult *result,
                           gpointer      user_data)
 {
+  ScopeCreatedData *data = user_data;
   GError *error = NULL;
 
   g_dbus_connection_call_finish (G_DBUS_CONNECTION (object), result, &error);
@@ -2827,10 +2833,50 @@ systemd_scope_created_cb (GObject      *object,
     }
 
   /* Unblock the waiting wrapper binary. */
-  close (GPOINTER_TO_INT (user_data));
+  close (data->fd);
+
+  if (data->task)
+    {
+      gint pending;
+      pending = GPOINTER_TO_INT (g_object_get_data ((GObject*) data->task,
+                                                    "pending-calls"));
+      pending -= 1;
+      g_object_set_data ((GObject*) data->task,
+                         "pending-calls",
+                         GINT_TO_POINTER (pending));
+
+      if (pending == 0 && !g_task_get_completed (data->task))
+        g_task_return_boolean (data->task, TRUE);
+    }
+
+  g_clear_object (&data->task);
+  g_free (data);
 }
 #endif
 
+static void
+launch_uris_with_spawn_flush_cb (GObject      *object,
+                                 GAsyncResult *result,
+                                 gpointer      user_data)
+{
+  GTask *task = G_TASK (user_data);
+  gint pending;
+
+  g_dbus_connection_flush_finish (G_DBUS_CONNECTION (object), result, NULL);
+
+  pending = GPOINTER_TO_INT (g_object_get_data ((GObject*) task,
+                                                "pending-calls"));
+  pending -= 1;
+  g_object_set_data ((GObject*) task,
+                     "pending-calls",
+                     GINT_TO_POINTER (pending));
+
+  if (pending == 0 && !g_task_get_completed (task))
+    g_task_return_boolean (task, TRUE);
+
+  g_object_unref (task);
+}
+
 static gboolean
 g_desktop_app_info_launch_uris_with_spawn (GDesktopAppInfo            *info,
                                            GDBusConnection            *session_bus,
@@ -2845,9 +2891,10 @@ g_desktop_app_info_launch_uris_with_spawn (GDesktopAppInfo            *info,
                                            gint                        stdin_fd,
                                            gint                        stdout_fd,
                                            gint                        stderr_fd,
-                                           GError                    **error)
+                                           GTask                      *task,
+                                           GError                    **error_out)
 {
-  gboolean completed = FALSE;
+  GError *error = NULL;
   GList *old_uris;
   GList *dup_uris;
 
@@ -2857,8 +2904,13 @@ g_desktop_app_info_launch_uris_with_spawn (GDesktopAppInfo            *info,
   char *sn_id = NULL;
   int argc;
 
+  /* We may get a task to report back on or an error. But never both. */
+  g_assert (!(task && error_out));
   g_return_val_if_fail (info != NULL, FALSE);
 
+  if (session_bus && task)
+    g_object_set_data ((GObject*) task, "pending-calls", GINT_TO_POINTER (0));
+
   if (launch_context)
     envp = g_app_launch_context_get_environment (launch_context);
   else
@@ -2881,8 +2933,8 @@ g_desktop_app_info_launch_uris_with_spawn (GDesktopAppInfo            *info,
       gpointer             setup_data = user_setup_data;
 
       old_uris = dup_uris;
-      if (!expand_application_parameters (info, exec_line, &dup_uris, &argc, &argv, error))
-        return FALSE;
+      if (!expand_application_parameters (info, exec_line, &dup_uris, &argc, &argv, &error))
+        goto out;
 
       /* Get the subset of URIs we're launching with this process */
       for (iter = old_uris; iter != NULL && iter != dup_uris; iter = iter->next)
@@ -2891,9 +2943,9 @@ g_desktop_app_info_launch_uris_with_spawn (GDesktopAppInfo            *info,
 
       if (info->terminal && !prepend_terminal_to_vector (&argc, &argv))
         {
-          g_set_error_literal (error, G_IO_ERROR, G_IO_ERROR_FAILED,
-                               _("Unable to find terminal required for application"));
-          return FALSE;
+          error = g_error_new_literal (G_IO_ERROR, G_IO_ERROR_FAILED,
+                                       _("Unable to find terminal required for application"));
+          goto out;
         }
 
       if (info->filename)
@@ -2950,9 +3002,9 @@ g_desktop_app_info_launch_uris_with_spawn (GDesktopAppInfo            *info,
        * otherwise our wrapper script will close both sides. */
       if (!g_unix_open_pipe (wrapper_data.pipe, 0, NULL))
         {
-          g_set_error_literal (error, G_IO_ERROR, G_IO_ERROR_FAILED,
-                               _("Unable to create pipe for systemd synchronization"));
-          return FALSE;
+          error = g_error_new_literal (G_IO_ERROR, G_IO_ERROR_FAILED,
+                                       _("Unable to create pipe for systemd synchronization"));
+          goto out;
         }
 
       /* Set CLOEXEC on the write pipe, so we don't need to deal with it in the child. */
@@ -2989,7 +3041,7 @@ g_desktop_app_info_launch_uris_with_spawn (GDesktopAppInfo            *info,
                                    stdin_fd,
                                    stdout_fd,
                                    stderr_fd,
-                                   error))
+                                   &error))
         {
 #if defined(__linux__) && !defined(__BIONIC__)
           close (wrapper_data.pipe[0]);
@@ -3002,7 +3054,7 @@ g_desktop_app_info_launch_uris_with_spawn (GDesktopAppInfo            *info,
           g_free (sn_id);
           g_list_free (launched_uris);
 
-          goto err;
+          goto out;
         }
 
 #if defined(__linux__) && !defined(__BIONIC__)
@@ -3011,11 +3063,32 @@ g_desktop_app_info_launch_uris_with_spawn (GDesktopAppInfo            *info,
       close (wrapper_data.pipe[0]);
 
       if (session_bus)
-        create_systemd_scope (session_bus,
-                              info,
-                              pid,
-                              systemd_scope_created_cb,
-                              GINT_TO_POINTER (wrapper_data.pipe[1]));
+        {
+          ScopeCreatedData *data;
+
+          data = g_new0 (ScopeCreatedData, 1);
+
+          if (task)
+            {
+              gint pending;
+              pending = GPOINTER_TO_INT (g_object_get_data ((GObject*) task,
+                                                            "pending-calls"));
+              pending += 1;
+              g_object_set_data ((GObject*) task,
+                                 "pending-calls",
+                                 GINT_TO_POINTER (pending));
+
+              data->task = g_object_ref (task);
+            }
+
+          data->fd = wrapper_data.pipe[1];
+
+          create_systemd_scope (session_bus,
+                                info,
+                                pid,
+                                systemd_scope_created_cb,
+                                data);
+        }
       else
         close (wrapper_data.pipe[1]);
 #endif
@@ -3050,8 +3123,6 @@ g_desktop_app_info_launch_uris_with_spawn (GDesktopAppInfo            *info,
     }
   while (dup_uris != NULL);
 
-  completed = TRUE;
-
 out:
   g_strfreev (argv);
   g_strfreev (envp);
@@ -3059,7 +3130,55 @@ out:
   g_list_free (launched_uris);
   g_free (sn_id);
 
-  return completed;
+  if (!error)
+    {
+      if (session_bus && task)
+        {
+          GCancellable *cancellable = g_task_get_cancellable (task);
+          gint pending;
+          pending = GPOINTER_TO_INT (g_object_get_data ((GObject*) task,
+                                                        "pending-calls"));
+          pending += 1;
+          g_object_set_data ((GObject*) task,
+                             "pending-calls",
+                             GINT_TO_POINTER (pending));
+
+          /* FIXME: The D-Bus message from the notify_desktop_launch() function
+           * can be still lost even if flush is called later. See:
+           * https://gitlab.freedesktop.org/dbus/dbus/issues/72
+           */
+          g_dbus_connection_flush (session_bus,
+                                   cancellable,
+                                   launch_uris_with_spawn_flush_cb,
+                                   g_steal_pointer (&task));
+        }
+      else if (session_bus)
+        {
+          /* No task available. */
+          g_dbus_connection_flush (session_bus,
+                                   NULL,
+                                   NULL,
+                                   NULL);
+        }
+      else if (task)
+        {
+          /* Return the given task. */
+          g_task_return_boolean (task, TRUE);
+          g_object_unref (task);
+        }
+    }
+  else
+    {
+      if (task)
+        {
+          g_task_return_error (task, error);
+          g_object_unref (task);
+        }
+      else
+        g_propagate_error (error_out, error);
+    }
+
+  return !error;
 }
 
 static gchar *
@@ -3192,7 +3311,7 @@ g_desktop_app_info_launch_uris_internal (GAppInfo                   *appinfo,
                                          GError                     **error)
 {
   GDesktopAppInfo *info = G_DESKTOP_APP_INFO (appinfo);
-  GDBusConnection *session_bus;
+  GDBusConnection *session_bus = NULL;
   gboolean success = TRUE;
 
   session_bus = g_bus_get_sync (G_BUS_TYPE_SESSION, NULL, NULL);
@@ -3208,17 +3327,9 @@ g_desktop_app_info_launch_uris_internal (GAppInfo                   *appinfo,
     success = g_desktop_app_info_launch_uris_with_spawn (info, session_bus, info->exec, uris, launch_context,
                                                          spawn_flags, user_setup, user_setup_data,
                                                          pid_callback, pid_callback_data,
-                                                         stdin_fd, stdout_fd, stderr_fd, error);
+                                                         stdin_fd, stdout_fd, stderr_fd, NULL, error);
 
-  if (session_bus != NULL)
-    {
-      /* This asynchronous flush holds a reference until it completes,
-       * which ensures that the following unref won't immediately kill
-       * the connection if we were the initial owner.
-       */
-      g_dbus_connection_flush (session_bus, NULL, NULL, NULL);
-      g_object_unref (session_bus);
-    }
+  g_object_unref (session_bus);
 
   return success;
 }
@@ -3272,18 +3383,6 @@ launch_uris_with_dbus_cb (GObject      *object,
   g_object_unref (task);
 }
 
-static void
-launch_uris_flush_cb (GObject      *object,
-                      GAsyncResult *result,
-                      gpointer      user_data)
-{
-  GTask *task = G_TASK (user_data);
-
-  g_dbus_connection_flush_finish (G_DBUS_CONNECTION (object), result, NULL);
-  g_task_return_boolean (task, TRUE);
-  g_object_unref (task);
-}
-
 static void
 launch_uris_bus_get_cb (GObject      *object,
                         GAsyncResult *result,
@@ -3294,7 +3393,6 @@ launch_uris_bus_get_cb (GObject      *object,
   LaunchUrisData *data = g_task_get_task_data (task);
   GCancellable *cancellable = g_task_get_cancellable (task);
   GDBusConnection *session_bus;
-  GError *error = NULL;
 
   session_bus = g_bus_get_finish (result, NULL);
 
@@ -3312,25 +3410,11 @@ launch_uris_bus_get_cb (GObject      *object,
     }
   else
     {
-      /* FIXME: The D-Bus message from the notify_desktop_launch() function
-       * can be still lost even if flush is called later. See:
-       * https://gitlab.freedesktop.org/dbus/dbus/issues/72
-       */
       g_desktop_app_info_launch_uris_with_spawn (info, session_bus, info->exec,
                                                  data->uris, data->context,
                                                  _SPAWN_FLAGS_DEFAULT, NULL,
                                                  NULL, NULL, NULL, -1, -1, -1,
-                                                 &error);
-      if (error != NULL)
-        {
-          g_task_return_error (task, g_steal_pointer (&error));
-          g_object_unref (task);
-        }
-      else
-        g_dbus_connection_flush (session_bus,
-                                 cancellable,
-                                 launch_uris_flush_cb,
-                                 g_steal_pointer (&task));
+                                                 g_steal_pointer (&task), NULL);
     }
 
   g_clear_object (&session_bus);
@@ -5115,7 +5199,7 @@ g_desktop_app_info_launch_action (GDesktopAppInfo   *info,
                                   const gchar       *action_name,
                                   GAppLaunchContext *launch_context)
 {
-  GDBusConnection *session_bus;
+  GDBusConnection *session_bus = NULL;
 
   g_return_if_fail (G_IS_DESKTOP_APP_INFO (info));
   g_return_if_fail (action_name != NULL);
@@ -5147,16 +5231,12 @@ g_desktop_app_info_launch_action (GDesktopAppInfo   *info,
       if (exec_line)
         g_desktop_app_info_launch_uris_with_spawn (info, session_bus, exec_line, NULL, launch_context,
                                                    _SPAWN_FLAGS_DEFAULT, NULL, NULL, NULL, NULL,
-                                                   -1, -1, -1, NULL);
+                                                   -1, -1, -1, NULL, NULL);
 
       g_free (exec_line);
     }
 
-  if (session_bus != NULL)
-    {
-      g_dbus_connection_flush (session_bus, NULL, NULL, NULL);
-      g_object_unref (session_bus);
-    }
+  g_object_unref (session_bus);
 }
 /* Epilogue {{{1 */
 


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]