[gnome-keyring] daemon: Use GLib unix signal handling

[Stefan Walter <stefw src gnome org>]

commit 5a8275348a250824491ccf559f9192a1abb38fc3
Author: Stef Walter <stefw gnome org>
Date:   Thu Mar 6 09:43:25 2014 +0100

    daemon: Use GLib unix signal handling
    
    Rather than our own home rolled version.

 daemon/control/gkd-control-server.c |   15 ++++-
 daemon/gkd-main.c                   |  113 +++++++----------------------------
 daemon/test-shutdown.c              |   32 ++++++++++
 3 files changed, 67 insertions(+), 93 deletions(-)
---
diff --git a/daemon/control/gkd-control-server.c b/daemon/control/gkd-control-server.c
index 9966e97..5a81eb4 100644
--- a/daemon/control/gkd-control-server.c
+++ b/daemon/control/gkd-control-server.c
@@ -241,8 +241,19 @@ control_process (EggBuffer *req, GIOChannel *channel)
        if (cdata) {
                g_return_if_fail (!egg_buffer_has_error (&cdata->buffer));
                egg_buffer_set_uint32 (&cdata->buffer, 0, cdata->buffer.len);
-               g_io_add_watch_full (channel, G_PRIORITY_DEFAULT, G_IO_OUT | G_IO_HUP,
-                                    control_output, cdata, control_data_free);
+
+               /* Can't send response in main loop, send here */
+               if (op == GKD_CONTROL_OP_QUIT) {
+                       if (write (g_io_channel_unix_get_fd (channel),
+                                  cdata->buffer.buf, cdata->buffer.len) != cdata->buffer.len)
+                               g_message ("couldn't write response to close control request");
+                       control_data_free (cdata);
+
+               /* Any other response, send in the main loop */
+               } else {
+                       g_io_add_watch_full (channel, G_PRIORITY_DEFAULT, G_IO_OUT | G_IO_HUP,
+                                            control_output, cdata, control_data_free);
+               }
        }
 }
 
diff --git a/daemon/gkd-main.c b/daemon/gkd-main.c
index 6664c28..15728a5 100644
--- a/daemon/gkd-main.c
+++ b/daemon/gkd-main.c
@@ -61,6 +61,7 @@
 #include <glib.h>
 #include <glib/gi18n.h>
 #include <glib-object.h>
+#include <glib-unix.h>
 
 #include <gcrypt.h>
 
@@ -81,8 +82,6 @@ typedef int socklen_t;
 
 EGG_SECURE_DECLARE (daemon_main);
 
-static GMainLoop *loop = NULL;
-
 /* -----------------------------------------------------------------------------
  * COMMAND LINE
  */
@@ -127,8 +126,7 @@ static gchar* login_password = NULL;
 static gchar* control_directory = NULL;
 static guint timeout_id = 0;
 static gboolean initialization_completed = FALSE;
-static gboolean sig_thread_valid = FALSE;
-static pthread_t sig_thread;
+static GMainLoop *loop = NULL;
 
 static GOptionEntry option_entries[] = {
        { "start", 's', 0, G_OPTION_ARG_NONE, &run_for_start,
@@ -396,96 +394,27 @@ dump_diagnostics (void)
  * SIGNALS
  */
 
-static sigset_t signal_set;
-static gint signal_quitting = 0;
-
-static gpointer
-signal_thread (gpointer user_data)
-{
-       GMainLoop *loop = user_data;
-       int sig, err;
-
-       for (;;) {
-               err = sigwait (&signal_set, &sig);
-               if (err != EINTR && err != 0) {
-                       g_warning ("couldn't wait for signals: %s", g_strerror (err));
-                       return NULL;
-               }
-
-               switch (sig) {
-               case SIGUSR1:
-#ifdef WITH_DEBUG
-                       dump_diagnostics ();
-#endif /* WITH_DEBUG */
-                       break;
-               case SIGPIPE:
-                       /* Ignore */
-                       break;
-               case SIGHUP:
-               case SIGTERM:
-                       g_atomic_int_set (&signal_quitting, 1);
-                       g_main_loop_quit (loop);
-                       return NULL;
-               default:
-                       g_warning ("received unexpected signal when waiting for signals: %d", sig);
-                       break;
-               }
-       }
-
-       g_assert_not_reached ();
-       return NULL;
-}
-
-static void
-setup_signal_handling (GMainLoop *loop)
-{
-       int res;
-
-       /*
-        * Block these signals for this thread, and any threads
-        * started up after this point (so essentially all threads).
-        *
-        * We also start a signal handling thread which uses signal_set
-        * to catch the various signals we're interested in.
-        */
-
-       sigemptyset (&signal_set);
-       sigaddset (&signal_set, SIGPIPE);
-       sigaddset (&signal_set, SIGHUP);
-       sigaddset (&signal_set, SIGTERM);
-       sigaddset (&signal_set, SIGUSR1);
-       pthread_sigmask (SIG_BLOCK, &signal_set, NULL);
-
-       res = pthread_create (&sig_thread, NULL, signal_thread, loop);
-       if (res == 0) {
-               sig_thread_valid = TRUE;
-       } else {
-               g_warning ("couldn't startup thread for signal handling: %s",
-                          g_strerror (res));
-       }
-}
-
 void
 gkd_main_quit (void)
 {
-       /*
-        * Send a signal to terminate our signal thread,
-        * which in turn runs stops the main loop and that
-        * starts the shutdown process.
-        */
+       g_main_loop_quit (loop);
+}
 
-       if (sig_thread_valid)
-               pthread_kill (sig_thread, SIGTERM);
-       else
-               raise (SIGTERM);
+static gboolean
+on_signal_term (gpointer user_data)
+{
+       gkd_main_quit ();
+       g_debug ("received signal, terminating");
+       return FALSE;
 }
 
-static void
-cleanup_signal_handling (void)
+static gboolean
+on_signal_usr1 (gpointer user_data)
 {
-       /* The thread is not joinable, so cleans itself up */
-       if (!g_atomic_int_get (&signal_quitting))
-               g_warning ("gkr_daemon_quit() was not used to shutdown the daemon");
+#ifdef WITH_DEBUG
+       dump_diagnostics ();
+#endif
+       return TRUE;
 }
 
 /* -----------------------------------------------------------------------------
@@ -1018,10 +947,14 @@ main (int argc, char *argv[])
                        cleanup_and_exit (1);
        }
 
+       signal (SIGPIPE, SIG_IGN);
+
        /* The whole forking and daemonizing dance starts here. */
        fork_and_print_environment();
 
-       setup_signal_handling (loop);
+       g_unix_signal_add (SIGTERM, on_signal_term, loop);
+       g_unix_signal_add (SIGHUP, on_signal_term, loop);
+       g_unix_signal_add (SIGUSR1, on_signal_usr1, loop);
 
        /* Prepare logging a second time, since we may be in a different process */
        prepare_logging();
@@ -1035,10 +968,8 @@ main (int argc, char *argv[])
        /* This wraps everything up in order */
        egg_cleanup_perform ();
 
-       /* Wrap up signal handling here */
-       cleanup_signal_handling ();
-
        g_free (control_directory);
 
+       g_debug ("exiting cleanly");
        return 0;
 }
diff --git a/daemon/test-shutdown.c b/daemon/test-shutdown.c
index b2fdb0c..28f6a9e 100644
--- a/daemon/test-shutdown.c
+++ b/daemon/test-shutdown.c
@@ -72,6 +72,36 @@ teardown (Test *test,
 }
 
 static void
+test_sigterm (Test *test,
+              gconstpointer unused)
+{
+       const gchar *argv[] = {
+               BUILDDIR "/gnome-keyring-daemon", "--foreground",
+               "--components=secrets,pkcs11", NULL
+       };
+
+       const gchar *control;
+       gchar **output;
+       gint status;
+       GPid pid;
+
+       output = gkd_test_launch_daemon (test->directory, argv, &pid, NULL);
+
+       control = g_environ_getenv (output, "GNOME_KEYRING_CONTROL");
+       g_assert_cmpstr (control, !=, NULL);
+
+       g_assert (gkd_control_unlock (control, "booo"));
+       g_strfreev (output);
+
+       /* Terminate the daemon */
+       g_assert_cmpint (kill (pid, SIGTERM), ==, 0);
+
+       /* Daemon should exit cleanly */
+       g_assert_cmpint (waitpid (pid, &status, 0), ==, pid);
+       g_assert_cmpint (status, ==, 0);
+}
+
+static void
 test_close_connection (Test *test,
                        gconstpointer unused)
 {
@@ -110,6 +140,8 @@ main (int argc, char **argv)
 
        g_test_add ("/daemon/shutdown/dbus-connection", Test, NULL,
                    setup, test_close_connection, teardown);
+       g_test_add ("/daemon/shutdown/sigterm", Test, NULL,
+                   setup, test_sigterm, teardown);
 
        return g_test_run ();
 }