mirror of
				https://gitlab.gnome.org/GNOME/glib.git
				synced 2025-10-31 00:12:19 +01:00 
			
		
		
		
	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.
This commit is contained in:
		
				
					committed by
					
						 Michael Catanzaro
						Michael Catanzaro
					
				
			
			
				
	
			
			
			
						parent
						
							ee61859c22
						
					
				
				
					commit
					35d24b2fa6
				
			| @@ -2849,11 +2849,17 @@ create_systemd_scope (GDBusConnection    *session_bus, | |||||||
|   g_free (unit_name); |   g_free (unit_name); | ||||||
| } | } | ||||||
|  |  | ||||||
|  | typedef struct { | ||||||
|  |   GTask *task; | ||||||
|  |   int fd; | ||||||
|  | } ScopeCreatedData; | ||||||
|  |  | ||||||
| static void | static void | ||||||
| systemd_scope_created_cb (GObject      *object, | systemd_scope_created_cb (GObject      *object, | ||||||
|                           GAsyncResult *result, |                           GAsyncResult *result, | ||||||
|                           gpointer      user_data) |                           gpointer      user_data) | ||||||
| { | { | ||||||
|  |   ScopeCreatedData *data = user_data; | ||||||
|   GVariant *res = NULL; |   GVariant *res = NULL; | ||||||
|   GError *error = NULL; |   GError *error = NULL; | ||||||
|  |  | ||||||
| @@ -2865,13 +2871,47 @@ systemd_scope_created_cb (GObject      *object, | |||||||
|     } |     } | ||||||
|  |  | ||||||
|   /* Unblock the waiting wrapper binary. */ |   /* Unblock the waiting wrapper binary. */ | ||||||
|   close (GPOINTER_TO_INT (user_data)); |  | ||||||
|  |   close (data->fd); | ||||||
|  |  | ||||||
|  |   if (data->task) | ||||||
|  |     { | ||||||
|  |       gint pending; | ||||||
|  |       pending = GPOINTER_TO_INT (g_task_get_task_data (data->task)); | ||||||
|  |       pending -= 1; | ||||||
|  |       g_task_set_task_data (data->task, GINT_TO_POINTER (pending), NULL); | ||||||
|  |  | ||||||
|  |       if (pending == 0 && !g_task_get_completed (data->task)) | ||||||
|  |         g_task_return_boolean (data->task, TRUE); | ||||||
|  |     } | ||||||
|  |  | ||||||
|   if (res) |   if (res) | ||||||
|     g_variant_unref (res); |     g_variant_unref (res); | ||||||
|  |   g_clear_object (&data->task); | ||||||
|  |   g_free (data); | ||||||
| } | } | ||||||
| #endif | #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_task_get_task_data (task)); | ||||||
|  |   pending -= 1; | ||||||
|  |   g_task_set_task_data (task, GINT_TO_POINTER (pending), NULL); | ||||||
|  |  | ||||||
|  |   if (pending == 0 && !g_task_get_completed (task)) | ||||||
|  |     g_task_return_boolean (task, TRUE); | ||||||
|  |  | ||||||
|  |   g_object_unref (task); | ||||||
|  | } | ||||||
|  |  | ||||||
| static gboolean | static gboolean | ||||||
| g_desktop_app_info_launch_uris_with_spawn (GDesktopAppInfo            *info, | g_desktop_app_info_launch_uris_with_spawn (GDesktopAppInfo            *info, | ||||||
|                                            GDBusConnection            *session_bus, |                                            GDBusConnection            *session_bus, | ||||||
| @@ -2886,9 +2926,10 @@ g_desktop_app_info_launch_uris_with_spawn (GDesktopAppInfo            *info, | |||||||
|                                            gint                        stdin_fd, |                                            gint                        stdin_fd, | ||||||
|                                            gint                        stdout_fd, |                                            gint                        stdout_fd, | ||||||
|                                            gint                        stderr_fd, |                                            gint                        stderr_fd, | ||||||
|                                            GError                    **error) |                                            GTask                      *task, | ||||||
|  |                                            GError                    **error_out) | ||||||
| { | { | ||||||
|   gboolean completed = FALSE; |   GError *error = NULL; | ||||||
|   GList *old_uris; |   GList *old_uris; | ||||||
|   GList *dup_uris; |   GList *dup_uris; | ||||||
|  |  | ||||||
| @@ -2898,8 +2939,15 @@ g_desktop_app_info_launch_uris_with_spawn (GDesktopAppInfo            *info, | |||||||
|   char *sn_id = NULL; |   char *sn_id = NULL; | ||||||
|   int argc; |   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); |   g_return_val_if_fail (info != NULL, FALSE); | ||||||
|  |  | ||||||
|  |   /* Surrounding code must not have set any data on the task | ||||||
|  |    * (it is cleared before calling this function). */ | ||||||
|  |   if (session_bus && task) | ||||||
|  |     g_assert (g_task_get_task_data (task) == NULL); | ||||||
|  |  | ||||||
|   if (launch_context) |   if (launch_context) | ||||||
|     envp = g_app_launch_context_get_environment (launch_context); |     envp = g_app_launch_context_get_environment (launch_context); | ||||||
|   else |   else | ||||||
| @@ -2922,8 +2970,8 @@ g_desktop_app_info_launch_uris_with_spawn (GDesktopAppInfo            *info, | |||||||
|       gpointer             setup_data = user_setup_data; |       gpointer             setup_data = user_setup_data; | ||||||
|  |  | ||||||
|       old_uris = dup_uris; |       old_uris = dup_uris; | ||||||
|       if (!expand_application_parameters (info, exec_line, &dup_uris, &argc, &argv, error)) |       if (!expand_application_parameters (info, exec_line, &dup_uris, &argc, &argv, &error)) | ||||||
|         return FALSE; |         goto out; | ||||||
|  |  | ||||||
|       /* Get the subset of URIs we're launching with this process */ |       /* Get the subset of URIs we're launching with this process */ | ||||||
|       for (iter = old_uris; iter != NULL && iter != dup_uris; iter = iter->next) |       for (iter = old_uris; iter != NULL && iter != dup_uris; iter = iter->next) | ||||||
| @@ -2932,9 +2980,9 @@ g_desktop_app_info_launch_uris_with_spawn (GDesktopAppInfo            *info, | |||||||
|  |  | ||||||
|       if (info->terminal && !prepend_terminal_to_vector (&argc, &argv)) |       if (info->terminal && !prepend_terminal_to_vector (&argc, &argv)) | ||||||
|         { |         { | ||||||
|           g_set_error_literal (error, G_IO_ERROR, G_IO_ERROR_FAILED, |           error = g_error_new_literal (G_IO_ERROR, G_IO_ERROR_FAILED, | ||||||
|                                        _("Unable to find terminal required for application")); |                                        _("Unable to find terminal required for application")); | ||||||
|           return FALSE; |           goto out; | ||||||
|         } |         } | ||||||
|  |  | ||||||
|       if (info->filename) |       if (info->filename) | ||||||
| @@ -2991,9 +3039,9 @@ g_desktop_app_info_launch_uris_with_spawn (GDesktopAppInfo            *info, | |||||||
|        * otherwise our wrapper script will close both sides. */ |        * otherwise our wrapper script will close both sides. */ | ||||||
|       if (!g_unix_open_pipe (wrapper_data.pipe, 0, NULL)) |       if (!g_unix_open_pipe (wrapper_data.pipe, 0, NULL)) | ||||||
|         { |         { | ||||||
|           g_set_error_literal (error, G_IO_ERROR, G_IO_ERROR_FAILED, |           error = g_error_new_literal (G_IO_ERROR, G_IO_ERROR_FAILED, | ||||||
|                                        _("Unable to create pipe for systemd synchronization")); |                                        _("Unable to create pipe for systemd synchronization")); | ||||||
|           return FALSE; |           goto out; | ||||||
|         } |         } | ||||||
|  |  | ||||||
|       /* Set CLOEXEC on the write pipe, so we don't need to deal with it in the child. */ |       /* Set CLOEXEC on the write pipe, so we don't need to deal with it in the child. */ | ||||||
| @@ -3030,7 +3078,7 @@ g_desktop_app_info_launch_uris_with_spawn (GDesktopAppInfo            *info, | |||||||
|                                    stdin_fd, |                                    stdin_fd, | ||||||
|                                    stdout_fd, |                                    stdout_fd, | ||||||
|                                    stderr_fd, |                                    stderr_fd, | ||||||
|                                    error)) |                                    &error)) | ||||||
|         { |         { | ||||||
| #if defined(__linux__) && !defined(__BIONIC__) | #if defined(__linux__) && !defined(__BIONIC__) | ||||||
|           close (wrapper_data.pipe[0]); |           close (wrapper_data.pipe[0]); | ||||||
| @@ -3049,11 +3097,29 @@ g_desktop_app_info_launch_uris_with_spawn (GDesktopAppInfo            *info, | |||||||
|       close (wrapper_data.pipe[0]); |       close (wrapper_data.pipe[0]); | ||||||
|  |  | ||||||
|       if (session_bus) |       if (session_bus) | ||||||
|  |         { | ||||||
|  |           ScopeCreatedData *data; | ||||||
|  |  | ||||||
|  |           data = g_new0 (ScopeCreatedData, 1); | ||||||
|  |  | ||||||
|  |           if (task) | ||||||
|  |             { | ||||||
|  |               gint pending; | ||||||
|  |               pending = GPOINTER_TO_INT (g_task_get_task_data (task)); | ||||||
|  |               pending += 1; | ||||||
|  |               g_task_set_task_data (task, GINT_TO_POINTER (pending), NULL); | ||||||
|  |  | ||||||
|  |               data->task = g_object_ref (task); | ||||||
|  |             } | ||||||
|  |  | ||||||
|  |           data->fd = wrapper_data.pipe[1]; | ||||||
|  |  | ||||||
|           create_systemd_scope (session_bus, |           create_systemd_scope (session_bus, | ||||||
|                                 info, |                                 info, | ||||||
|                                 pid, |                                 pid, | ||||||
|                                 systemd_scope_created_cb, |                                 systemd_scope_created_cb, | ||||||
|                               GINT_TO_POINTER (wrapper_data.pipe[1])); |                                 data); | ||||||
|  |         } | ||||||
|       else |       else | ||||||
|         close (wrapper_data.pipe[1]); |         close (wrapper_data.pipe[1]); | ||||||
| #endif | #endif | ||||||
| @@ -3088,8 +3154,6 @@ g_desktop_app_info_launch_uris_with_spawn (GDesktopAppInfo            *info, | |||||||
|     } |     } | ||||||
|   while (dup_uris != NULL); |   while (dup_uris != NULL); | ||||||
|  |  | ||||||
|   completed = TRUE; |  | ||||||
|  |  | ||||||
| out: | out: | ||||||
|   g_strfreev (argv); |   g_strfreev (argv); | ||||||
|   g_strfreev (envp); |   g_strfreev (envp); | ||||||
| @@ -3097,7 +3161,52 @@ out: | |||||||
|   g_list_free (launched_uris); |   g_list_free (launched_uris); | ||||||
|   g_free (sn_id); |   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_task_get_task_data (task)); | ||||||
|  |           pending += 1; | ||||||
|  |           g_task_set_task_data (task, GINT_TO_POINTER (pending), NULL); | ||||||
|  |  | ||||||
|  |           /* 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 * | static gchar * | ||||||
| @@ -3246,17 +3355,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, |     success = g_desktop_app_info_launch_uris_with_spawn (info, session_bus, info->exec, uris, launch_context, | ||||||
|                                                          spawn_flags, user_setup, user_setup_data, |                                                          spawn_flags, user_setup, user_setup_data, | ||||||
|                                                          pid_callback, pid_callback_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) |   g_clear_object (&session_bus); | ||||||
|     { |  | ||||||
|       /* 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); |  | ||||||
|     } |  | ||||||
|  |  | ||||||
|   return success; |   return success; | ||||||
| } | } | ||||||
| @@ -3310,18 +3411,6 @@ launch_uris_with_dbus_cb (GObject      *object, | |||||||
|   g_object_unref (task); |   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 | static void | ||||||
| launch_uris_bus_get_cb (GObject      *object, | launch_uris_bus_get_cb (GObject      *object, | ||||||
|                         GAsyncResult *result, |                         GAsyncResult *result, | ||||||
| @@ -3330,12 +3419,20 @@ launch_uris_bus_get_cb (GObject      *object, | |||||||
|   GTask *task = G_TASK (user_data); |   GTask *task = G_TASK (user_data); | ||||||
|   GDesktopAppInfo *info = G_DESKTOP_APP_INFO (g_task_get_source_object (task)); |   GDesktopAppInfo *info = G_DESKTOP_APP_INFO (g_task_get_source_object (task)); | ||||||
|   LaunchUrisData *data = g_task_get_task_data (task); |   LaunchUrisData *data = g_task_get_task_data (task); | ||||||
|  |   LaunchUrisData *data_copy = NULL; | ||||||
|   GCancellable *cancellable = g_task_get_cancellable (task); |   GCancellable *cancellable = g_task_get_cancellable (task); | ||||||
|   GDBusConnection *session_bus; |   GDBusConnection *session_bus; | ||||||
|   GError *error = NULL; |  | ||||||
|  |  | ||||||
|   session_bus = g_bus_get_finish (result, NULL); |   session_bus = g_bus_get_finish (result, NULL); | ||||||
|  |  | ||||||
|  |   data_copy = g_new0 (LaunchUrisData, 1); | ||||||
|  |   data_copy->appinfo = g_steal_pointer (&data->appinfo); | ||||||
|  |   data_copy->uris = g_steal_pointer (&data->uris); | ||||||
|  |   data_copy->context = g_steal_pointer (&data->context); | ||||||
|  |  | ||||||
|  |   /* Allow other data to be attached to the task. */ | ||||||
|  |   g_task_set_task_data (task, NULL, NULL); | ||||||
|  |  | ||||||
|   if (session_bus && info->app_id) |   if (session_bus && info->app_id) | ||||||
|     { |     { | ||||||
|       /* FIXME: The g_document_portal_add_documents() function, which is called |       /* FIXME: The g_document_portal_add_documents() function, which is called | ||||||
| @@ -3343,34 +3440,21 @@ launch_uris_bus_get_cb (GObject      *object, | |||||||
|        * uses blocking calls. |        * uses blocking calls. | ||||||
|        */ |        */ | ||||||
|       g_desktop_app_info_launch_uris_with_dbus (info, session_bus, |       g_desktop_app_info_launch_uris_with_dbus (info, session_bus, | ||||||
|                                                 data->uris, data->context, |                                                 data_copy->uris, data_copy->context, | ||||||
|                                                 cancellable, |                                                 cancellable, | ||||||
|                                                 launch_uris_with_dbus_cb, |                                                 launch_uris_with_dbus_cb, | ||||||
|                                                 g_steal_pointer (&task)); |                                                 g_steal_pointer (&task)); | ||||||
|     } |     } | ||||||
|   else |   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, |       g_desktop_app_info_launch_uris_with_spawn (info, session_bus, info->exec, | ||||||
|                                                  data->uris, data->context, |                                                  data_copy->uris, data_copy->context, | ||||||
|                                                  _SPAWN_FLAGS_DEFAULT, NULL, |                                                  _SPAWN_FLAGS_DEFAULT, NULL, | ||||||
|                                                  NULL, NULL, NULL, -1, -1, -1, |                                                  NULL, NULL, NULL, -1, -1, -1, | ||||||
|                                                  &error); |                                                  g_steal_pointer (&task), NULL); | ||||||
|       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)); |  | ||||||
|     } |     } | ||||||
|  |  | ||||||
|  |   launch_uris_data_free (data_copy); | ||||||
|   g_clear_object (&session_bus); |   g_clear_object (&session_bus); | ||||||
| } | } | ||||||
|  |  | ||||||
| @@ -5186,16 +5270,12 @@ g_desktop_app_info_launch_action (GDesktopAppInfo   *info, | |||||||
|       if (exec_line) |       if (exec_line) | ||||||
|         g_desktop_app_info_launch_uris_with_spawn (info, session_bus, exec_line, NULL, launch_context, |         g_desktop_app_info_launch_uris_with_spawn (info, session_bus, exec_line, NULL, launch_context, | ||||||
|                                                    _SPAWN_FLAGS_DEFAULT, NULL, NULL, NULL, NULL, |                                                    _SPAWN_FLAGS_DEFAULT, NULL, NULL, NULL, NULL, | ||||||
|                                                    -1, -1, -1, NULL); |                                                    -1, -1, -1, NULL, NULL); | ||||||
|  |  | ||||||
|       g_free (exec_line); |       g_free (exec_line); | ||||||
|     } |     } | ||||||
|  |  | ||||||
|   if (session_bus != NULL) |   g_clear_object (&session_bus); | ||||||
|     { |  | ||||||
|       g_dbus_connection_flush (session_bus, NULL, NULL, NULL); |  | ||||||
|       g_object_unref (session_bus); |  | ||||||
|     } |  | ||||||
| } | } | ||||||
| /* Epilogue {{{1 */ | /* Epilogue {{{1 */ | ||||||
|  |  | ||||||
|   | |||||||
		Reference in New Issue
	
	Block a user