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

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);
 }