[gdm] server: Process SIGUSR1 more carefully

[Jasper St. Pierre <jstpierre src gnome org>]

commit 956d7d1c7a0cfbf2beacdb9e88e645e15ad32047
Author: Jasper St. Pierre <jstpierre mecheye net>
Date:   Fri Feb 14 14:32:50 2014 -0500

    server: Process SIGUSR1 more carefully
    
    If the slave is removed as a separate process, it means that we need
    to be more careful with our handling of SIGUSR1. If multiple X servers
    are launched at once, we need to use siginfo_t to get the PID of the
    thing that sent the user signal, and make sure we signal the correct
    GdmServer.
    
    glib doesn't have native siginfo support, so do it ourselves by using
    a worker thread that spins around waiting for sigwaitinfo.
    
    https://bugzilla.gnome.org/show_bug.cgi?id=724382

 daemon/gdm-server.c |   86 ++++++++++++++++++++++++++++++++++++++++++--------
 daemon/main.c       |   15 +++++++++
 2 files changed, 87 insertions(+), 14 deletions(-)
---
diff --git a/daemon/gdm-server.c b/daemon/gdm-server.c
index 1606b3a..fe012a2 100644
--- a/daemon/gdm-server.c
+++ b/daemon/gdm-server.c
@@ -94,7 +94,6 @@ struct GdmServerPrivate
         char    *auth_file;
 
         guint    child_watch_id;
-        guint    sigusr1_id;
 
         gboolean is_initial;
 };
@@ -180,16 +179,77 @@ gdm_server_get_display_device (GdmServer *server)
         return g_strdup (server->priv->display_device);
 }
 
+static void
+gdm_server_ready (GdmServer *server)
+{
+        g_debug ("GdmServer: Got USR1 from X server - emitting READY");
+        g_signal_emit (server, signals[READY], 0);
+}
+
+static GSList *active_servers;
+static gboolean sigusr1_thread_running;
+static GCond sigusr1_thread_cond;
+static GMutex sigusr1_thread_mutex;
+
 static gboolean
-on_sigusr1 (gpointer user_data)
+got_sigusr1 (gpointer user_data)
+{
+        GPid pid = GPOINTER_TO_UINT (user_data);
+        GSList *l;
+
+        g_debug ("GdmServer: got SIGUSR1 from PID %d", pid);
 
+        for (l = active_servers; l; l = l->next) {
+                GdmServer *server = l->data;
+
+                if (server->priv->pid == pid)
+                        gdm_server_ready (server);
+        }
+
+        return G_SOURCE_REMOVE;
+}
+
+static gpointer
+sigusr1_thread_main (gpointer user_data)
 {
-        GdmServer *server = user_data;
+        sigset_t sigusr1_mask;
 
-        g_debug ("GdmServer: Got USR1 from X server - emitting READY");
+        /* Handle only SIGUSR1 */
+        sigemptyset (&sigusr1_mask);
+        sigaddset (&sigusr1_mask, SIGUSR1);
+        sigprocmask (SIG_SETMASK, &sigusr1_mask, NULL);
 
-        g_signal_emit (server, signals[READY], 0);
-        return FALSE;
+        g_mutex_lock (&sigusr1_thread_mutex);
+        sigusr1_thread_running = TRUE;
+        g_cond_signal (&sigusr1_thread_cond);
+        g_mutex_unlock (&sigusr1_thread_mutex);
+
+        /* Spin waiting for a SIGUSR1 */
+        while (TRUE) {
+                siginfo_t info;
+
+                if (sigwaitinfo (&sigusr1_mask, &info) == -1)
+                        continue;
+
+                g_idle_add (got_sigusr1, GUINT_TO_POINTER (info.si_pid));
+        }
+
+        return NULL;
+}
+
+static void
+gdm_server_launch_sigusr1_thread_if_needed (void)
+{
+        static GThread *sigusr1_thread;
+
+        if (sigusr1_thread == NULL) {
+                sigusr1_thread = g_thread_new ("gdm SIGUSR1 catcher", sigusr1_thread_main, NULL);
+
+                g_mutex_lock (&sigusr1_thread_mutex);
+                while (!sigusr1_thread_running)
+                        g_cond_wait (&sigusr1_thread_cond, &sigusr1_thread_mutex);
+                g_mutex_unlock (&sigusr1_thread_mutex);
+        }
 }
 
 /* We keep a connection (parent_dsp) open with the parent X server
@@ -676,6 +736,8 @@ server_child_watch (GPid       pid,
                 g_signal_emit (server, signals [DIED], 0, num);
         }
 
+        active_servers = g_slist_remove (active_servers, server);
+
         g_spawn_close_pid (server->priv->pid);
         server->priv->pid = -1;
 
@@ -719,6 +781,10 @@ gdm_server_spawn (GdmServer    *server,
         g_debug ("GdmServer: Starting X server process: %s", freeme);
         g_free (freeme);
 
+        active_servers = g_slist_append (active_servers, server);
+
+        gdm_server_launch_sigusr1_thread_if_needed ();
+
         if (!g_spawn_async_with_pipes (NULL,
                                        argv,
                                        (char **)env->pdata,
@@ -1063,16 +1129,11 @@ gdm_server_class_init (GdmServerClass *klass)
 static void
 gdm_server_init (GdmServer *server)
 {
-
         server->priv = GDM_SERVER_GET_PRIVATE (server);
 
         server->priv->pid = -1;
 
         server->priv->log_dir = g_strdup (LOGDIR);
-
-        server->priv->sigusr1_id = g_unix_signal_add (SIGUSR1,
-                                                      on_sigusr1,
-                                                      server);
 }
 
 static void
@@ -1087,9 +1148,6 @@ gdm_server_finalize (GObject *object)
 
         g_return_if_fail (server->priv != NULL);
 
-        if (server->priv->sigusr1_id > 0)
-                g_source_remove (server->priv->sigusr1_id);
-
         gdm_server_stop (server);
 
         g_free (server->priv->command);
diff --git a/daemon/main.c b/daemon/main.c
index d008aac..cdb41fa 100644
--- a/daemon/main.c
+++ b/daemon/main.c
@@ -295,6 +295,19 @@ is_debug_set (void)
         return debug;
 }
 
+/* SIGUSR1 is used by the X server to tell us that we're ready, so
+ * block it. We'll unblock it in the worker thread in gdm-server.c
+ */
+static void
+block_sigusr1 (void)
+{
+        sigset_t mask;
+
+        sigemptyset (&mask);
+        sigaddset (&mask, SIGUSR1);
+        sigprocmask (SIG_BLOCK, &mask, NULL);
+}
+
 int
 main (int    argc,
       char **argv)
@@ -315,6 +328,8 @@ main (int    argc,
                 { NULL }
         };
 
+        block_sigusr1 ();
+
         bindtextdomain (GETTEXT_PACKAGE, GNOMELOCALEDIR);
         textdomain (GETTEXT_PACKAGE);
         setlocale (LC_ALL, "");