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

2 files changed, 87 insertions, 14 deletions

diff --git a/daemon/gdm-server.c b/daemon/gdm-server.c
index 1606b3af..fe012a26 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 d008aacb..cdb41fa2 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, "");