[gdm] daemon: don't forcible kill pam after 3 seconds

[Ray Strode <halfline src gnome org>]

commit d5aa5a1e0e2b2648acafa5534c75fc0ab0e4055b
Author: Ray Strode <rstrode redhat com>
Date:   Fri Sep 9 00:51:38 2011 -0400

    daemon: don't forcible kill pam after 3 seconds
    
    Right now when stopping a conversation we give it 3 seconds
    to die and then kill kill kill.
    
    This commit changes the killing to be asynchronous and not
    time out until absolutely necessary.

 daemon/gdm-session-direct.c     |  105 ++++++++++++++++++++++++++++----------
 daemon/gdm-session-worker-job.c |   27 ++++++----
 daemon/gdm-session-worker-job.h |    3 +-
 daemon/gdm-welcome-session.c    |   17 ++++++-
 4 files changed, 111 insertions(+), 41 deletions(-)
---
diff --git a/daemon/gdm-session-direct.c b/daemon/gdm-session-direct.c
index c25bc9b..e178985 100644
--- a/daemon/gdm-session-direct.c
+++ b/daemon/gdm-session-direct.c
@@ -75,6 +75,7 @@ typedef struct
         char                *service_name;
         DBusConnection      *worker_connection;
         DBusMessage         *message_pending_reply;
+        guint32              is_stopping : 1;
 } GdmSessionConversation;
 
 struct _GdmSessionDirectPrivate
@@ -1784,6 +1785,8 @@ worker_exited (GdmSessionWorkerJob *job,
                int                  code,
                GdmSessionConversation *conversation)
 {
+        GdmSessionDirect *impl = GDM_SESSION_DIRECT (conversation->session);
+
         g_debug ("GdmSessionDirect: Worker job exited: %d", code);
 
         g_object_ref (conversation->job);
@@ -1791,10 +1794,19 @@ worker_exited (GdmSessionWorkerJob *job,
                 _gdm_session_session_exited (GDM_SESSION (conversation->session), code);
         }
 
+        g_hash_table_steal (impl->priv->conversations, conversation->service_name);
+
         g_debug ("GdmSessionDirect: Emitting conversation-stopped signal");
         _gdm_session_conversation_stopped (GDM_SESSION (conversation->session),
                                            conversation->service_name);
         g_object_unref (conversation->job);
+
+        if (conversation->is_stopping) {
+                g_object_unref (conversation->job);
+                conversation->job = NULL;
+        }
+
+        free_conversation (conversation);
 }
 
 static void
@@ -1802,6 +1814,8 @@ worker_died (GdmSessionWorkerJob *job,
              int                  signum,
              GdmSessionConversation *conversation)
 {
+        GdmSessionDirect *impl = GDM_SESSION_DIRECT (conversation->session);
+
         g_debug ("GdmSessionDirect: Worker job died: %d", signum);
 
         g_object_ref (conversation->job);
@@ -1809,10 +1823,19 @@ worker_died (GdmSessionWorkerJob *job,
                 _gdm_session_session_died (GDM_SESSION (conversation->session), signum);
         }
 
+        g_hash_table_steal (impl->priv->conversations, conversation->service_name);
+
         g_debug ("GdmSessionDirect: Emitting conversation-stopped signal");
         _gdm_session_conversation_stopped (GDM_SESSION (conversation->session),
                                            conversation->service_name);
         g_object_unref (conversation->job);
+
+        if (conversation->is_stopping) {
+                g_object_unref (conversation->job);
+                conversation->job = NULL;
+        }
+
+        free_conversation (conversation);
 }
 
 static GdmSessionConversation *
@@ -1842,14 +1865,14 @@ start_conversation (GdmSessionDirect *session,
                           conversation);
 
         job_name = g_strdup_printf ("gdm-session-worker [pam/%s]", service_name);
-        if (!gdm_session_worker_job_start (conversation->job,
-                                           job_name)) {
+        if (!gdm_session_worker_job_start (conversation->job, job_name)) {
                 g_object_unref (conversation->job);
                 g_free (conversation->service_name);
                 g_free (conversation);
                 g_free (job_name);
                 return NULL;
         }
+
         g_free (job_name);
 
         conversation->worker_pid = gdm_session_worker_job_get_pid (conversation->job);
@@ -1864,16 +1887,6 @@ stop_conversation (GdmSessionConversation *conversation)
 
         session = conversation->session;
 
-        g_signal_handlers_disconnect_by_func (conversation->job,
-                                              G_CALLBACK (worker_started),
-                                              conversation);
-        g_signal_handlers_disconnect_by_func (conversation->job,
-                                              G_CALLBACK (worker_exited),
-                                              conversation);
-        g_signal_handlers_disconnect_by_func (conversation->job,
-                                              G_CALLBACK (worker_died),
-                                              conversation);
-
         if (conversation->worker_connection != NULL) {
                 dbus_connection_remove_filter (conversation->worker_connection, on_message, session);
 
@@ -1881,14 +1894,27 @@ stop_conversation (GdmSessionConversation *conversation)
                 conversation->worker_connection = NULL;
         }
 
+        conversation->is_stopping = TRUE;
         gdm_session_worker_job_stop (conversation->job);
+}
+
+static void
+stop_conversation_now (GdmSessionConversation *conversation)
+{
+        GdmSessionDirect *session;
 
+        session = conversation->session;
+
+        if (conversation->worker_connection != NULL) {
+                dbus_connection_remove_filter (conversation->worker_connection, on_message, session);
+
+                dbus_connection_close (conversation->worker_connection);
+                conversation->worker_connection = NULL;
+        }
+
+        gdm_session_worker_job_stop_now (conversation->job);
         g_object_unref (conversation->job);
         conversation->job = NULL;
-
-        g_debug ("GdmSessionDirect: Emitting conversation-stopped signal");
-        _gdm_session_conversation_stopped (GDM_SESSION (session),
-                                           conversation->service_name);
 }
 
 static void
@@ -1900,6 +1926,20 @@ gdm_session_direct_start_conversation (GdmSession *session,
 
         g_return_if_fail (session != NULL);
 
+        conversation = g_hash_table_lookup (impl->priv->conversations,
+                                            service_name);
+
+        if (conversation != NULL) {
+                if (!conversation->is_stopping) {
+                        g_warning ("GdmSessionDirect: conversation %s started more than once", service_name);
+                        return;
+                }
+                g_debug ("GdmSessionDirect: stopping old conversation %s", service_name);
+                gdm_session_worker_job_stop_now (conversation->job);
+                g_object_unref (conversation->job);
+                conversation->job = NULL;
+        }
+
         g_debug ("GdmSessionDirect: starting conversation %s", service_name);
 
         conversation = start_conversation (impl, service_name);
@@ -1923,7 +1963,6 @@ gdm_session_direct_stop_conversation (GdmSession *session,
 
         if (conversation != NULL) {
                 stop_conversation (conversation);
-                g_hash_table_remove (impl->priv->conversations, service_name);
         }
 }
 
@@ -2358,7 +2397,8 @@ gdm_session_direct_open_session (GdmSession *session,
 
 static void
 stop_all_other_conversations (GdmSessionDirect        *session,
-                              GdmSessionConversation  *conversation_to_keep)
+                              GdmSessionConversation  *conversation_to_keep,
+                              gboolean                 now)
 {
         GHashTableIter iter;
         gpointer key, value;
@@ -2381,20 +2421,29 @@ stop_all_other_conversations (GdmSessionDirect        *session,
                 conversation = (GdmSessionConversation *) value;
 
                 if (conversation == conversation_to_keep) {
-                        g_hash_table_iter_steal (&iter);
-                        g_free (key);
+                        if (now) {
+                                g_hash_table_iter_steal (&iter);
+                                g_free (key);
+                        }
                 } else {
-                        stop_conversation (conversation);
+                        if (now) {
+                                stop_conversation_now (conversation);
+                        } else {
+                                stop_conversation (conversation);
+                        }
                 }
         }
 
-        g_hash_table_remove_all (session->priv->conversations);
+        if (now) {
+                g_hash_table_remove_all (session->priv->conversations);
 
-        if (conversation_to_keep != NULL) {
-                g_hash_table_insert (session->priv->conversations,
-                                     g_strdup (conversation_to_keep->service_name),
-                                     conversation_to_keep);
+                if (conversation_to_keep != NULL) {
+                        g_hash_table_insert (session->priv->conversations,
+                                             g_strdup (conversation_to_keep->service_name),
+                                             conversation_to_keep);
+                }
         }
+
 }
 
 static void
@@ -2417,7 +2466,7 @@ gdm_session_direct_start_session (GdmSession *session,
                 return;
         }
 
-        stop_all_other_conversations (impl, conversation);
+        stop_all_other_conversations (impl, conversation, FALSE);
 
         if (impl->priv->selected_program == NULL) {
                 command = get_session_command (impl);
@@ -2443,7 +2492,7 @@ gdm_session_direct_start_session (GdmSession *session,
 static void
 stop_all_conversations (GdmSessionDirect *session)
 {
-        stop_all_other_conversations (session, NULL);
+        stop_all_other_conversations (session, NULL, TRUE);
 }
 
 static void
diff --git a/daemon/gdm-session-worker-job.c b/daemon/gdm-session-worker-job.c
index a129353..ac96cf1 100644
--- a/daemon/gdm-session-worker-job.c
+++ b/daemon/gdm-session-worker-job.c
@@ -284,7 +284,7 @@ gdm_session_worker_job_start (GdmSessionWorkerJob *session_worker_job,
 }
 
 static void
-session_worker_job_died (GdmSessionWorkerJob *session_worker_job)
+handle_session_worker_job_death (GdmSessionWorkerJob *session_worker_job)
 {
         int exit_status;
 
@@ -303,13 +303,11 @@ session_worker_job_died (GdmSessionWorkerJob *session_worker_job)
         g_debug ("GdmSessionWorkerJob: SessionWorkerJob died");
 }
 
-gboolean
-gdm_session_worker_job_stop (GdmSessionWorkerJob *session_worker_job)
+void
+gdm_session_worker_job_stop_now (GdmSessionWorkerJob *session_worker_job)
 {
-        int res;
-
         if (session_worker_job->priv->pid <= 1) {
-                return TRUE;
+                return;
         }
 
         /* remove watch source before we can wait on child */
@@ -318,17 +316,26 @@ gdm_session_worker_job_stop (GdmSessionWorkerJob *session_worker_job)
                 session_worker_job->priv->child_watch_id = 0;
         }
 
+        gdm_session_worker_job_stop (session_worker_job);
+        handle_session_worker_job_death (session_worker_job);
+}
+
+void
+gdm_session_worker_job_stop (GdmSessionWorkerJob *session_worker_job)
+{
+        int res;
+
+        if (session_worker_job->priv->pid <= 1) {
+                return;
+        }
+
         g_debug ("GdmSessionWorkerJob: Stopping job pid:%d", session_worker_job->priv->pid);
 
         res = gdm_signal_pid (session_worker_job->priv->pid, SIGTERM);
 
         if (res < 0) {
                 g_warning ("Unable to kill session worker process");
-        } else {
-                session_worker_job_died (session_worker_job);
         }
-
-        return TRUE;
 }
 
 GPid
diff --git a/daemon/gdm-session-worker-job.h b/daemon/gdm-session-worker-job.h
index 4833f23..ab102a9 100644
--- a/daemon/gdm-session-worker-job.h
+++ b/daemon/gdm-session-worker-job.h
@@ -59,7 +59,8 @@ void                    gdm_session_worker_job_set_server_address (GdmSessionWor
                                                                    const char          *server_address);
 gboolean                gdm_session_worker_job_start              (GdmSessionWorkerJob *session_worker_job,
                                                                    const char          *name);
-gboolean                gdm_session_worker_job_stop               (GdmSessionWorkerJob *session_worker_job);
+void                    gdm_session_worker_job_stop               (GdmSessionWorkerJob *session_worker_job);
+void                    gdm_session_worker_job_stop_now           (GdmSessionWorkerJob *session_worker_job);
 
 GPid                    gdm_session_worker_job_get_pid            (GdmSessionWorkerJob *session_worker_job);
 
diff --git a/daemon/gdm-welcome-session.c b/daemon/gdm-welcome-session.c
index 63000f1..2bcc3d0 100644
--- a/daemon/gdm-welcome-session.c
+++ b/daemon/gdm-welcome-session.c
@@ -799,6 +799,15 @@ on_conversation_started (GdmSession        *session,
         g_free (log_path);
 }
 
+static void
+on_conversation_stopped (GdmSession        *session,
+                         const char        *service_name,
+                         GdmWelcomeSession *welcome_session)
+{
+        g_debug ("GdmWelcomeSession: conversation stopped");
+        stop_dbus_daemon (welcome_session);
+}
+
 /**
  * gdm_welcome_session_start:
  * @disp: Pointer to a GdmDisplay structure
@@ -828,6 +837,10 @@ gdm_welcome_session_start (GdmWelcomeSession *welcome_session)
                           "conversation-started",
                           G_CALLBACK (on_conversation_started),
                           welcome_session);
+        g_signal_connect (GDM_SESSION (welcome_session->priv->session),
+                          "conversation-stopped",
+                          G_CALLBACK (on_conversation_stopped),
+                          welcome_session);
         g_signal_connect (welcome_session->priv->session,
                           "setup-complete",
                           G_CALLBACK (on_session_setup_complete),
@@ -878,10 +891,10 @@ gdm_welcome_session_stop (GdmWelcomeSession *welcome_session)
 
                 g_object_unref (welcome_session->priv->session);
                 welcome_session->priv->session = NULL;
+        } else {
+                stop_dbus_daemon (welcome_session);
         }
 
-        stop_dbus_daemon (welcome_session);
-
         return TRUE;
 }