daemon: stop greeter session not greeter worker

Since changing the greeter to run in its own PAM
session, we've started killing the worker process
instead of the session process right before the
user's session is started.

This is wrong, because it means we don't give the
greeter session time to kill itself in an orderly
way before we rip away its session bus.

This commit changes the welcome session stop function
to kill the pid of the welcome session instead of
the pid of its worker parent. This change requires
reinstrumenting all callers of gdm_welcome_session_stop
to not free the session until the "stopped" signal
following gdm_welcome_session_stop.

https://bugzilla.gnome.org/show_bug.cgi?id=658481

4 files changed, 85 insertions, 41 deletions

diff --git a/daemon/gdm-factory-slave.c b/daemon/gdm-factory-slave.c
index c61f8f43..6154cd3a 100644
--- a/daemon/gdm-factory-slave.c
+++ b/daemon/gdm-factory-slave.c
@@ -117,6 +117,9 @@ on_greeter_session_stop (GdmGreeterSession *greeter,
                          GdmFactorySlave   *slave)
 {
         g_debug ("GdmFactorySlave: Greeter stopped");
+
+        g_object_unref (GDM_FACTORY_SLAVE (slave)->priv->greeter);
+        GDM_FACTORY_SLAVE (slave)->priv->greeter = NULL;
 }
 
 static void
@@ -847,8 +850,6 @@ gdm_factory_slave_stop (GdmSlave *slave)
 
         if (GDM_FACTORY_SLAVE (slave)->priv->greeter != NULL) {
                 gdm_welcome_session_stop (GDM_WELCOME_SESSION (GDM_FACTORY_SLAVE (slave)->priv->greeter));
-                g_object_unref (GDM_FACTORY_SLAVE (slave)->priv->greeter);
-                GDM_FACTORY_SLAVE (slave)->priv->greeter = NULL;
         }
 
         if (GDM_FACTORY_SLAVE (slave)->priv->server != NULL) {
diff --git a/daemon/gdm-simple-slave.c b/daemon/gdm-simple-slave.c
index 2f0163f3..2f38bf60 100644
--- a/daemon/gdm-simple-slave.c
+++ b/daemon/gdm-simple-slave.c
@@ -212,7 +212,9 @@ on_session_exited (GdmSession     *session,
                    GdmSimpleSlave *slave)
 {
         g_debug ("GdmSimpleSlave: session exited with code %d\n", exit_code);
-        gdm_slave_stopped (GDM_SLAVE (slave));
+        if (slave->priv->start_session_service_name == NULL) {
+                gdm_slave_stopped (GDM_SLAVE (slave));
+        }
 }
 
 static void
@@ -224,7 +226,9 @@ on_session_died (GdmSession     *session,
                  signal_number,
                  g_strsignal (signal_number));
 
-        gdm_slave_stopped (GDM_SLAVE (slave));
+        if (slave->priv->start_session_service_name == NULL) {
+                gdm_slave_stopped (GDM_SLAVE (slave));
+        }
 }
 
 static gboolean
@@ -443,17 +447,12 @@ stop_greeter (GdmSimpleSlave *slave)
         g_free (username);
 
         gdm_welcome_session_stop (GDM_WELCOME_SESSION (slave->priv->greeter));
-        gdm_greeter_server_stop (slave->priv->greeter_server);
-
-        g_object_unref (slave->priv->greeter);
-        slave->priv->greeter = NULL;
 }
 
 static gboolean
 start_session_timeout (GdmSimpleSlave *slave)
 {
 
-        char    *auth_file;
         gboolean migrated;
 
         g_debug ("GdmSimpleSlave: accredited");
@@ -469,28 +468,14 @@ start_session_timeout (GdmSimpleSlave *slave)
                    user switching. */
                 queue_greeter_reset (slave);
 
-                goto out;
+                slave->priv->start_session_id = 0;
+                g_free (slave->priv->start_session_service_name);
+                slave->priv->start_session_service_name = NULL;
+        } else {
+                /* Session actually gets started from on_greeter_session_stopped */
+                stop_greeter (slave);
         }
 
-        stop_greeter (slave);
-
-        auth_file = NULL;
-        add_user_authorization (slave, &auth_file);
-
-        g_assert (auth_file != NULL);
-
-        g_object_set (slave->priv->session,
-                      "user-x11-authority-file", auth_file,
-                      NULL);
-
-        g_free (auth_file);
-
-        gdm_session_start_session (GDM_SESSION (slave->priv->session),
-                                   slave->priv->start_session_service_name);
- out:
-        slave->priv->start_session_id = 0;
-        g_free (slave->priv->start_session_service_name);
-        slave->priv->start_session_service_name = NULL;
         return FALSE;
 }
 
@@ -983,7 +968,34 @@ on_greeter_session_stop (GdmGreeterSession *greeter,
                          GdmSimpleSlave    *slave)
 {
         g_debug ("GdmSimpleSlave: Greeter stopped");
-        gdm_slave_stopped (GDM_SLAVE (slave));
+        if (slave->priv->start_session_service_name == NULL) {
+                gdm_slave_stopped (GDM_SLAVE (slave));
+        } else {
+                gdm_greeter_server_stop (slave->priv->greeter_server);
+
+                char    *auth_file;
+                auth_file = NULL;
+                add_user_authorization (slave, &auth_file);
+
+                g_assert (auth_file != NULL);
+
+                g_object_set (slave->priv->session,
+                              "user-x11-authority-file", auth_file,
+                              NULL);
+
+                g_free (auth_file);
+
+                gdm_session_start_session (GDM_SESSION (slave->priv->session),
+                                           slave->priv->start_session_service_name);
+
+                slave->priv->start_session_id = 0;
+                g_free (slave->priv->start_session_service_name);
+                slave->priv->start_session_service_name = NULL;
+        }
+
+        g_object_unref (slave->priv->greeter);
+        slave->priv->greeter = NULL;
+
 }
 
 static void
@@ -992,7 +1004,9 @@ on_greeter_session_exited (GdmGreeterSession    *greeter,
                            GdmSimpleSlave       *slave)
 {
         g_debug ("GdmSimpleSlave: Greeter exited: %d", code);
-        gdm_slave_stopped (GDM_SLAVE (slave));
+        if (slave->priv->start_session_service_name == NULL) {
+                gdm_slave_stopped (GDM_SLAVE (slave));
+        }
 }
 
 static void
diff --git a/daemon/gdm-welcome-session.c b/daemon/gdm-welcome-session.c
index 2bcc3d08..41144a87 100644
--- a/daemon/gdm-welcome-session.c
+++ b/daemon/gdm-welcome-session.c
@@ -60,6 +60,7 @@ struct GdmWelcomeSessionPrivate
 {
         GdmSession     *session;
         char           *command;
+        GPid            pid;
 
         char           *user_name;
         char           *group_name;
@@ -762,6 +763,7 @@ on_session_started (GdmSession        *session,
                     int                pid,
                     GdmWelcomeSession *welcome_session)
 {
+        welcome_session->priv->pid = pid;
         g_signal_emit (G_OBJECT (welcome_session), signals [STARTED], 0);
 }
 
@@ -770,6 +772,9 @@ on_session_exited (GdmSession        *session,
                    int                exit_code,
                    GdmWelcomeSession *welcome_session)
 {
+        gdm_session_stop_conversation (GDM_SESSION (welcome_session->priv->session),
+                                       "gdm-welcome");
+
         g_signal_emit (G_OBJECT (welcome_session), signals [EXITED], 0, exit_code);
 }
 
@@ -778,6 +783,9 @@ on_session_died (GdmSession        *session,
                  int                signal_number,
                  GdmWelcomeSession *welcome_session)
 {
+        gdm_session_stop_conversation (GDM_SESSION (welcome_session->priv->session),
+                                       "gdm-welcome");
+
         g_signal_emit (G_OBJECT (welcome_session), signals [DIED], 0, signal_number);
 }
 
@@ -804,8 +812,22 @@ on_conversation_stopped (GdmSession        *session,
                          const char        *service_name,
                          GdmWelcomeSession *welcome_session)
 {
+        GdmSession *conversation_session;
+
+        conversation_session = welcome_session->priv->session;
+        welcome_session->priv->session = NULL;
+
         g_debug ("GdmWelcomeSession: conversation stopped");
         stop_dbus_daemon (welcome_session);
+
+        if (welcome_session->priv->pid > 1) {
+                g_signal_emit (G_OBJECT (welcome_session), signals [STOPPED], 0);
+        }
+
+        if (conversation_session != NULL) {
+                gdm_session_close (GDM_SESSION (conversation_session));
+                g_object_unref (conversation_session);
+        }
 }
 
 /**
@@ -884,15 +906,21 @@ gdm_welcome_session_start (GdmWelcomeSession *welcome_session)
 gboolean
 gdm_welcome_session_stop (GdmWelcomeSession *welcome_session)
 {
-        if (welcome_session->priv->session != NULL) {
-                gdm_session_stop_conversation (GDM_SESSION (welcome_session->priv->session),
-                                               "gdm-welcome");
-                gdm_session_close (GDM_SESSION (welcome_session->priv->session));
-
-                g_object_unref (welcome_session->priv->session);
-                welcome_session->priv->session = NULL;
+        if (welcome_session->priv->pid > 1) {
+                gdm_signal_pid (welcome_session->priv->pid, SIGTERM);
         } else {
-                stop_dbus_daemon (welcome_session);
+                if (welcome_session->priv->session != NULL) {
+                        gdm_session_stop_conversation (GDM_SESSION (welcome_session->priv->session),
+                                                       "gdm-welcome");
+                        gdm_session_close (GDM_SESSION (welcome_session->priv->session));
+
+                        g_object_unref (welcome_session->priv->session);
+                        welcome_session->priv->session = NULL;
+                } else {
+                        stop_dbus_daemon (welcome_session);
+                }
+
+                g_signal_emit (G_OBJECT (welcome_session), signals [STOPPED], 0);
         }
 
         return TRUE;
diff --git a/daemon/gdm-xdmcp-chooser-slave.c b/daemon/gdm-xdmcp-chooser-slave.c
index 23d9e4fd..fee36303 100644
--- a/daemon/gdm-xdmcp-chooser-slave.c
+++ b/daemon/gdm-xdmcp-chooser-slave.c
@@ -99,6 +99,9 @@ on_chooser_session_stop (GdmChooserSession    *chooser,
 {
         g_debug ("GdmXdmcpChooserSlave: Chooser stopped");
         gdm_slave_stopped (GDM_SLAVE (slave));
+
+        g_object_unref (GDM_XDMCP_CHOOSER_SLAVE (slave)->priv->chooser);
+        GDM_XDMCP_CHOOSER_SLAVE (slave)->priv->chooser = NULL;
 }
 
 static void
@@ -308,8 +311,6 @@ gdm_xdmcp_chooser_slave_stop (GdmSlave *slave)
 
         if (GDM_XDMCP_CHOOSER_SLAVE (slave)->priv->chooser != NULL) {
                 gdm_welcome_session_stop (GDM_WELCOME_SESSION (GDM_XDMCP_CHOOSER_SLAVE (slave)->priv->chooser));
-                g_object_unref (GDM_XDMCP_CHOOSER_SLAVE (slave)->priv->chooser);
-                GDM_XDMCP_CHOOSER_SLAVE (slave)->priv->chooser = NULL;
         }
 
         return TRUE;