From 46fadc83c114540c0ec0adb191e0a8f7a7d897c7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jo=C3=A3o=20Paulo=20Rechi=20Vita?= Date: Fri, 10 Jul 2015 12:52:04 -0400 Subject: [PATCH 1/3] Make gdm-session-worker exit cleanly Calling gdm_session_stop_conversation() in gdm_launch_environment_stop() sends a SIGTERM to gdm-session-worker without waiting for it to die. The next step is calling gdm_session_close() to close the session, which stops all conversations of that session object, sending a 2nd SIGTERM to gdm-session-worker, this time waiting on its PID. On gdm-session-worker side, the first SIGTERM is caught by on_shutdown_signal(), its custom SIGTERM handler, which quits the mainloop and unrefs the worker object. Quiting the mainloop replaces the custom SIGTERM handler with the system default one (exit immediately). During the worker object class finalization gdm-session-worker may receive the 2nd SIGTERM, which leads to its immediate termination, without waiting for its children, which in turn leads to the main gdm process exit. Since systemd relies on the SIGCHLD from the main gdm process to tell when the service has stopped, this behavior breaks any unit that has a Conflicts=gdm.service entry and relies on the X server not being around when it is started. This commit removes the call to gdm_session_stop_conversation() in gdm_launch_environment_stop() and leaves it to be stopped in gdm_session_close(). [endlessm/eos-shell#4921] https://bugzilla.gnome.org/show_bug.cgi?id=752388 --- daemon/gdm-launch-environment.c | 1 - 1 file changed, 1 deletion(-) diff --git a/daemon/gdm-launch-environment.c b/daemon/gdm-launch-environment.c index 4aee187..af3bf87 100644 --- a/daemon/gdm-launch-environment.c +++ b/daemon/gdm-launch-environment.c @@ -402,61 +402,60 @@ gdm_launch_environment_start (GdmLaunchEnvironment *launch_environment) gdm_session_start_conversation (launch_environment->priv->session, "gdm-launch-environment"); if (launch_environment->priv->dbus_session_bus_address) { gdm_session_select_program (launch_environment->priv->session, launch_environment->priv->command); } else { /* wrap it in dbus-launch */ char *command = g_strdup_printf ("%s %s", DBUS_LAUNCH_COMMAND, launch_environment->priv->command); gdm_session_select_program (launch_environment->priv->session, command); g_free (command); } res = TRUE; out: if (local_error) { g_critical ("GdmLaunchEnvironment: %s", local_error->message); g_clear_error (&local_error); } return res; } gboolean gdm_launch_environment_stop (GdmLaunchEnvironment *launch_environment) { if (launch_environment->priv->pid > 1) { gdm_signal_pid (-launch_environment->priv->pid, SIGTERM); } if (launch_environment->priv->session != NULL) { - gdm_session_stop_conversation (launch_environment->priv->session, "gdm-launch-environment"); gdm_session_close (launch_environment->priv->session); g_clear_object (&launch_environment->priv->session); } g_signal_emit (G_OBJECT (launch_environment), signals [STOPPED], 0); return TRUE; } GdmSession * gdm_launch_environment_get_session (GdmLaunchEnvironment *launch_environment) { return launch_environment->priv->session; } char * gdm_launch_environment_get_session_id (GdmLaunchEnvironment *launch_environment) { return g_strdup (launch_environment->priv->session_id); } static void _gdm_launch_environment_set_verification_mode (GdmLaunchEnvironment *launch_environment, GdmSessionVerificationMode verification_mode) { launch_environment->priv->verification_mode = verification_mode; } static void -- 2.3.7 From 92564b47a85f9a308f7bfc34b8017f2767bf4677 Mon Sep 17 00:00:00 2001 From: Ray Strode Date: Wed, 4 Mar 2015 11:12:24 -0500 Subject: [PATCH 2/3] manager: clean up manager in dispose not finalize Seems more appropriate. https://bugzilla.gnome.org/show_bug.cgi?id=745975 --- daemon/gdm-manager.c | 19 ++++++++++++------- 1 file changed, 12 insertions(+), 7 deletions(-) diff --git a/daemon/gdm-manager.c b/daemon/gdm-manager.c index f060135..e0af40c 100644 --- a/daemon/gdm-manager.c +++ b/daemon/gdm-manager.c @@ -75,61 +75,61 @@ struct GdmManagerPrivate GHashTable *transient_sessions; GHashTable *open_reauthentication_requests; gboolean xdmcp_enabled; GCancellable *cancellable; gboolean started; gboolean wait_for_go; gboolean show_local_greeter; GDBusProxy *bus_proxy; GDBusConnection *connection; GDBusObjectManagerServer *object_manager; }; enum { PROP_0, PROP_XDMCP_ENABLED, PROP_SHOW_LOCAL_GREETER }; enum { DISPLAY_ADDED, DISPLAY_REMOVED, LAST_SIGNAL }; static guint signals [LAST_SIGNAL] = { 0, }; static void gdm_manager_class_init (GdmManagerClass *klass); static void gdm_manager_init (GdmManager *manager); -static void gdm_manager_finalize (GObject *object); +static void gdm_manager_dispose (GObject *object); static void create_seed_session_for_display (GdmManager *manager, GdmDisplay *display, uid_t allowed_user); static void touch_ran_once_marker_file (GdmManager *manager); static gpointer manager_object = NULL; static void manager_interface_init (GdmDBusManagerIface *interface); G_DEFINE_TYPE_WITH_CODE (GdmManager, gdm_manager, GDM_DBUS_TYPE_MANAGER_SKELETON, G_IMPLEMENT_INTERFACE (GDM_DBUS_TYPE_MANAGER, manager_interface_init)); #ifdef WITH_SYSTEMD static char * get_session_id_for_pid_systemd (pid_t pid, GError **error) { char *session, *gsession; int ret; session = NULL; ret = sd_pid_get_session (pid, &session); if (ret < 0) { g_set_error (error, GDM_DISPLAY_ERROR, GDM_DISPLAY_ERROR_GETTING_SESSION_INFO, "Error getting session id from systemd: %s", @@ -2186,61 +2186,61 @@ gdm_manager_constructor (GType type, guint n_construct_properties, GObjectConstructParam *construct_properties) { GdmManager *manager; manager = GDM_MANAGER (G_OBJECT_CLASS (gdm_manager_parent_class)->constructor (type, n_construct_properties, construct_properties)); gdm_dbus_manager_set_version (GDM_DBUS_MANAGER (manager), PACKAGE_VERSION); manager->priv->local_factory = gdm_local_display_factory_new (manager->priv->display_store); #ifdef HAVE_LIBXDMCP if (manager->priv->xdmcp_enabled) { manager->priv->xdmcp_factory = gdm_xdmcp_display_factory_new (manager->priv->display_store); } #endif return G_OBJECT (manager); } static void gdm_manager_class_init (GdmManagerClass *klass) { GObjectClass *object_class = G_OBJECT_CLASS (klass); object_class->get_property = gdm_manager_get_property; object_class->set_property = gdm_manager_set_property; object_class->constructor = gdm_manager_constructor; - object_class->finalize = gdm_manager_finalize; + object_class->dispose = gdm_manager_dispose; signals [DISPLAY_ADDED] = g_signal_new ("display-added", G_TYPE_FROM_CLASS (object_class), G_SIGNAL_RUN_LAST, G_STRUCT_OFFSET (GdmManagerClass, display_added), NULL, NULL, g_cclosure_marshal_VOID__STRING, G_TYPE_NONE, 1, G_TYPE_STRING); signals [DISPLAY_REMOVED] = g_signal_new ("display-removed", G_TYPE_FROM_CLASS (object_class), G_SIGNAL_RUN_LAST, G_STRUCT_OFFSET (GdmManagerClass, display_removed), NULL, NULL, g_cclosure_marshal_VOID__STRING, G_TYPE_NONE, 1, G_TYPE_STRING); g_object_class_install_property (object_class, PROP_XDMCP_ENABLED, g_param_spec_boolean ("xdmcp-enabled", NULL, NULL, FALSE, G_PARAM_READWRITE | G_PARAM_CONSTRUCT)); @@ -2271,100 +2271,105 @@ gdm_manager_init (GdmManager *manager) "display-added", G_CALLBACK (on_display_added), manager); g_signal_connect (G_OBJECT (manager->priv->display_store), "display-removed", G_CALLBACK (on_display_removed), manager); } static void unexport_display (const char *id, GdmDisplay *display, GdmManager *manager) { if (!g_dbus_connection_is_closed (manager->priv->connection)) g_dbus_object_manager_server_unexport (manager->priv->object_manager, id); } static void finish_display (const char *id, GdmDisplay *display, GdmManager *manager) { if (gdm_display_get_status (display) == GDM_DISPLAY_MANAGED) gdm_display_unmanage (display); gdm_display_finish (display); } static void -gdm_manager_finalize (GObject *object) +gdm_manager_dispose (GObject *object) { GdmManager *manager; g_return_if_fail (object != NULL); g_return_if_fail (GDM_IS_MANAGER (object)); manager = GDM_MANAGER (object); g_return_if_fail (manager->priv != NULL); #ifdef HAVE_LIBXDMCP g_clear_object (&manager->priv->xdmcp_factory); #endif g_clear_object (&manager->priv->local_factory); - g_hash_table_unref (manager->priv->open_reauthentication_requests); - g_hash_table_unref (manager->priv->transient_sessions); + g_clear_pointer (&manager->priv->open_reauthentication_requests, + (GDestroyNotify) + g_hash_table_unref); + g_clear_pointer (&manager->priv->transient_sessions, + (GDestroyNotify) + g_hash_table_unref); + g_list_free_full (manager->priv->user_sessions, (GDestroyNotify) g_object_unref); manager->priv->user_sessions = NULL; g_signal_handlers_disconnect_by_func (G_OBJECT (manager->priv->display_store), G_CALLBACK (on_display_added), manager); g_signal_handlers_disconnect_by_func (G_OBJECT (manager->priv->display_store), G_CALLBACK (on_display_removed), manager); if (!g_dbus_connection_is_closed (manager->priv->connection)) { gdm_display_store_foreach (manager->priv->display_store, (GdmDisplayStoreFunc)unexport_display, manager); g_dbus_interface_skeleton_unexport (G_DBUS_INTERFACE_SKELETON (manager)); } gdm_display_store_foreach (manager->priv->display_store, (GdmDisplayStoreFunc) finish_display, manager); gdm_display_store_clear (manager->priv->display_store); g_dbus_object_manager_server_set_connection (manager->priv->object_manager, NULL); g_clear_object (&manager->priv->connection); g_clear_object (&manager->priv->object_manager); - g_object_unref (manager->priv->display_store); + g_clear_object (&manager->priv->display_store); - G_OBJECT_CLASS (gdm_manager_parent_class)->finalize (object); + G_OBJECT_CLASS (gdm_manager_parent_class)->dispose (object); } GdmManager * gdm_manager_new (void) { if (manager_object != NULL) { g_object_ref (manager_object); } else { gboolean res; manager_object = g_object_new (GDM_TYPE_MANAGER, NULL); g_object_add_weak_pointer (manager_object, (gpointer *) &manager_object); res = register_manager (manager_object); if (! res) { g_object_unref (manager_object); return NULL; } } return GDM_MANAGER (manager_object); } -- 2.3.7 From c6243ccc362cb51bb87043e18108d99117abd6c1 Mon Sep 17 00:00:00 2001 From: Ray Strode Date: Wed, 4 Mar 2015 11:17:05 -0500 Subject: [PATCH 3/3] manager: make sure to explicitly close user sessions in dispose We don't want ref count leaks to lead to unkilled sessions. https://bugzilla.gnome.org/show_bug.cgi?id=745975 --- daemon/gdm-manager.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/daemon/gdm-manager.c b/daemon/gdm-manager.c index e0af40c..12520ac 100644 --- a/daemon/gdm-manager.c +++ b/daemon/gdm-manager.c @@ -2293,60 +2293,63 @@ finish_display (const char *id, GdmManager *manager) { if (gdm_display_get_status (display) == GDM_DISPLAY_MANAGED) gdm_display_unmanage (display); gdm_display_finish (display); } static void gdm_manager_dispose (GObject *object) { GdmManager *manager; g_return_if_fail (object != NULL); g_return_if_fail (GDM_IS_MANAGER (object)); manager = GDM_MANAGER (object); g_return_if_fail (manager->priv != NULL); #ifdef HAVE_LIBXDMCP g_clear_object (&manager->priv->xdmcp_factory); #endif g_clear_object (&manager->priv->local_factory); g_clear_pointer (&manager->priv->open_reauthentication_requests, (GDestroyNotify) g_hash_table_unref); g_clear_pointer (&manager->priv->transient_sessions, (GDestroyNotify) g_hash_table_unref); + g_list_foreach (manager->priv->user_sessions, + (GFunc) gdm_session_close, + NULL); g_list_free_full (manager->priv->user_sessions, (GDestroyNotify) g_object_unref); manager->priv->user_sessions = NULL; g_signal_handlers_disconnect_by_func (G_OBJECT (manager->priv->display_store), G_CALLBACK (on_display_added), manager); g_signal_handlers_disconnect_by_func (G_OBJECT (manager->priv->display_store), G_CALLBACK (on_display_removed), manager); if (!g_dbus_connection_is_closed (manager->priv->connection)) { gdm_display_store_foreach (manager->priv->display_store, (GdmDisplayStoreFunc)unexport_display, manager); g_dbus_interface_skeleton_unexport (G_DBUS_INTERFACE_SKELETON (manager)); } gdm_display_store_foreach (manager->priv->display_store, (GdmDisplayStoreFunc) finish_display, manager); gdm_display_store_clear (manager->priv->display_store); g_dbus_object_manager_server_set_connection (manager->priv->object_manager, NULL); g_clear_object (&manager->priv->connection); g_clear_object (&manager->priv->object_manager); g_clear_object (&manager->priv->display_store); -- 2.3.7