daemon: Use GLib unix signal handling

Rather than our own home rolled version.

3 files changed, 67 insertions, 93 deletions

diff --git a/daemon/control/gkd-control-server.c b/daemon/control/gkd-control-server.c
index 9966e979..5a81eb49 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 6664c289..15728a59 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 b2fdb0cf..28f6a9e1 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 ();
 }