From 6d87866328db32a8274327751dff4895e35c7e40 Mon Sep 17 00:00:00 2001 From: Giovanni Campagna Date: Mon, 20 Aug 2012 22:47:19 +0200 Subject: slave: rework stop code paths Previously code would emit stopped signal directly, and rely on the side effect of main dropping the last reference to do the actual stop (including killing X). As this would break in case of memory leaks, move to calling stop when the session exits. Also, ensure that stop can be called multiple times (as it still called from finalize). https://bugzilla.gnome.org/show_bug.cgi?id=682571 --- daemon/gdm-simple-slave.c | 25 +++++++++++-------------- daemon/gdm-slave.c | 12 +++--------- daemon/gdm-slave.h | 1 - daemon/gdm-xdmcp-chooser-slave.c | 18 +++++++++++------- 4 files changed, 25 insertions(+), 31 deletions(-) diff --git a/daemon/gdm-simple-slave.c b/daemon/gdm-simple-slave.c index 36b93a49..cc5244b0 100644 --- a/daemon/gdm-simple-slave.c +++ b/daemon/gdm-simple-slave.c @@ -226,9 +226,7 @@ on_session_exited (GdmSession *session, g_object_set (GDM_SLAVE (slave), "session-id", NULL, NULL); g_debug ("GdmSimpleSlave: session exited with code %d\n", exit_code); - if (slave->priv->start_session_service_name == NULL) { - gdm_slave_stopped (GDM_SLAVE (slave)); - } + gdm_slave_stop (GDM_SLAVE (slave)); } static void @@ -242,10 +240,7 @@ on_session_died (GdmSession *session, g_debug ("GdmSimpleSlave: session died with signal %d, (%s)", signal_number, g_strsignal (signal_number)); - - if (slave->priv->start_session_service_name == NULL) { - gdm_slave_stopped (GDM_SLAVE (slave)); - } + gdm_slave_stop (GDM_SLAVE (slave)); } static gboolean @@ -685,7 +680,7 @@ on_session_client_disconnected (GdmSession *session, NULL); if ( ! display_is_local && !slave->priv->session_is_running) { - gdm_slave_stopped (GDM_SLAVE (slave)); + gdm_slave_stop (GDM_SLAVE (slave)); } } @@ -825,7 +820,7 @@ on_greeter_environment_session_stopped (GdmLaunchEnvironment *greeter_environmen { g_debug ("GdmSimpleSlave: Greeter stopped"); if (slave->priv->start_session_service_name == NULL) { - gdm_slave_stopped (GDM_SLAVE (slave)); + gdm_slave_stop (GDM_SLAVE (slave)); } else { start_session (slave); } @@ -841,7 +836,7 @@ on_greeter_environment_session_exited (GdmLaunchEnvironment *greeter_environm { g_debug ("GdmSimpleSlave: Greeter exited: %d", code); if (slave->priv->start_session_service_name == NULL) { - gdm_slave_stopped (GDM_SLAVE (slave)); + gdm_slave_stop (GDM_SLAVE (slave)); } } @@ -852,7 +847,7 @@ on_greeter_environment_session_died (GdmLaunchEnvironment *greeter_environmen { g_debug ("GdmSimpleSlave: Greeter died: %d", signal); if (slave->priv->start_session_service_name == NULL) { - gdm_slave_stopped (GDM_SLAVE (slave)); + gdm_slave_stop (GDM_SLAVE (slave)); } } @@ -1271,7 +1266,7 @@ on_server_exited (GdmServer *server, { g_debug ("GdmSimpleSlave: server exited with code %d\n", exit_code); - gdm_slave_stopped (GDM_SLAVE (slave)); + gdm_slave_stop (GDM_SLAVE (slave)); #ifdef WITH_PLYMOUTH if (slave->priv->plymouth_is_running) { @@ -1289,7 +1284,7 @@ on_server_died (GdmServer *server, signal_number, g_strsignal (signal_number)); - gdm_slave_stopped (GDM_SLAVE (slave)); + gdm_slave_stop (GDM_SLAVE (slave)); #ifdef WITH_PLYMOUTH if (slave->priv->plymouth_is_running) { @@ -1494,6 +1489,7 @@ gdm_simple_slave_stop (GdmSlave *slave) if (self->priv->greeter_environment != NULL) { stop_greeter (self); + self->priv->greeter_environment = NULL; } if (self->priv->session_is_running) { @@ -1512,6 +1508,7 @@ gdm_simple_slave_stop (GdmSlave *slave) gdm_simple_slave_revoke_console_permissions (self); #endif + self->priv->session_is_running = FALSE; } if (self->priv->session != NULL) { @@ -1572,7 +1569,7 @@ gdm_simple_slave_finalize (GObject *object) g_return_if_fail (slave->priv != NULL); - gdm_simple_slave_stop (GDM_SLAVE (slave)); + gdm_slave_stop (GDM_SLAVE (slave)); g_hash_table_unref (slave->priv->open_reauthentication_requests); diff --git a/daemon/gdm-slave.c b/daemon/gdm-slave.c index 18138e57..ea8bdb40 100644 --- a/daemon/gdm-slave.c +++ b/daemon/gdm-slave.c @@ -898,20 +898,14 @@ gdm_slave_stop (GdmSlave *slave) g_debug ("GdmSlave: stopping slave"); g_object_ref (slave); + ret = GDM_SLAVE_GET_CLASS (slave)->stop (slave); - g_object_unref (slave); + g_signal_emit (slave, signals [STOPPED], 0); + g_object_unref (slave); return ret; } -void -gdm_slave_stopped (GdmSlave *slave) -{ - g_return_if_fail (GDM_IS_SLAVE (slave)); - - g_signal_emit (slave, signals [STOPPED], 0); -} - gboolean gdm_slave_add_user_authorization (GdmSlave *slave, const char *username, diff --git a/daemon/gdm-slave.h b/daemon/gdm-slave.h index c9a40029..9478dd32 100644 --- a/daemon/gdm-slave.h +++ b/daemon/gdm-slave.h @@ -110,7 +110,6 @@ void gdm_slave_save_root_windows (GdmSlave *slave); gboolean gdm_slave_run_script (GdmSlave *slave, const char *dir, const char *username); -void gdm_slave_stopped (GdmSlave *slave); void gdm_slave_export_interface (GdmSlave *slave, GDBusInterfaceSkeleton *interface); G_END_DECLS diff --git a/daemon/gdm-xdmcp-chooser-slave.c b/daemon/gdm-xdmcp-chooser-slave.c index fef0ff80..444444cd 100644 --- a/daemon/gdm-xdmcp-chooser-slave.c +++ b/daemon/gdm-xdmcp-chooser-slave.c @@ -101,7 +101,7 @@ on_chooser_session_stop (GdmLaunchEnvironment *chooser, GdmXdmcpChooserSlave *slave) { g_debug ("GdmXdmcpChooserSlave: Chooser stopped"); - gdm_slave_stopped (GDM_SLAVE (slave)); + gdm_slave_stop (GDM_SLAVE (slave)); g_object_unref (GDM_XDMCP_CHOOSER_SLAVE (slave)->priv->chooser_environment); GDM_XDMCP_CHOOSER_SLAVE (slave)->priv->chooser_environment = NULL; @@ -116,7 +116,7 @@ on_chooser_session_exited (GdmLaunchEnvironment *chooser, g_object_set (GDM_SLAVE (slave), "session-id", NULL, NULL); - gdm_slave_stopped (GDM_SLAVE (slave)); + gdm_slave_stop (GDM_SLAVE (slave)); } static void @@ -128,7 +128,7 @@ on_chooser_session_died (GdmLaunchEnvironment *chooser, g_object_set (GDM_SLAVE (slave), "session-id", NULL, NULL); - gdm_slave_stopped (GDM_SLAVE (slave)); + gdm_slave_stop (GDM_SLAVE (slave)); } static void @@ -150,7 +150,7 @@ on_chooser_disconnected (GdmSession *session, /* stop pinging */ alarm (0); - gdm_slave_stopped (GDM_SLAVE (slave)); + gdm_slave_stop (GDM_SLAVE (slave)); } static void @@ -336,12 +336,16 @@ gdm_xdmcp_chooser_slave_start (GdmSlave *slave) static gboolean gdm_xdmcp_chooser_slave_stop (GdmSlave *slave) { + GdmXdmcpChooserSlave *self = GDM_XDMCP_CHOOSER_SLAVE (slave); + g_debug ("GdmXdmcpChooserSlave: Stopping xdmcp_chooser_slave"); GDM_SLAVE_CLASS (gdm_xdmcp_chooser_slave_parent_class)->stop (slave); - if (GDM_XDMCP_CHOOSER_SLAVE (slave)->priv->chooser_environment != NULL) { - gdm_launch_environment_stop (GDM_LAUNCH_ENVIRONMENT (GDM_XDMCP_CHOOSER_SLAVE (slave)->priv->chooser_environment)); + if (self->priv->chooser_environment != NULL) { + gdm_launch_environment_stop (GDM_LAUNCH_ENVIRONMENT (self->priv->chooser_environment)); + g_object_unref (self->priv->chooser_environment); + self->priv->chooser_environment = NULL; } return TRUE; @@ -428,7 +432,7 @@ gdm_xdmcp_chooser_slave_finalize (GObject *object) g_return_if_fail (xdmcp_chooser_slave->priv != NULL); - gdm_xdmcp_chooser_slave_stop (GDM_SLAVE (xdmcp_chooser_slave)); + gdm_slave_stop (GDM_SLAVE (xdmcp_chooser_slave)); G_OBJECT_CLASS (gdm_xdmcp_chooser_slave_parent_class)->finalize (object); } -- cgit v1.2.1