core: use GUnixSignalWatchSource to simplify signal handling

Replace the pthread_sigwait()-based signal handling with
g_unix_signal_add()-based handling, and get rid of all the
now-unnecessary calls to nm_unblock_posix_signals() when spawning
subprocesses.

As a bonus, this also fixes the "^C in gdb kills NM too" bug.

src/Makefile.am

@@ -320,8 +320,6 @@ nm_sources = \
 	nm-manager.h \
 	nm-policy.c \
 	nm-policy.h \
-	nm-posix-signals.c \
-	nm-posix-signals.h \
 	nm-properties-changed-signal.c \
 	nm-properties-changed-signal.h \
 	nm-rfkill-manager.c \
@@ -481,8 +479,6 @@ libnm_iface_helper_la_SOURCES = \
 	nm-enum-types.h \
 	nm-logging.c \
 	nm-logging.h \
-	nm-posix-signals.c \
-	nm-posix-signals.h \
 	NetworkManagerUtils.c \
 	NetworkManagerUtils.h
 

src/NetworkManagerUtils.c

@@ -47,7 +47,6 @@
 #include "nm-setting-wireless.h"
 #include "nm-setting-wireless-security.h"
 #include "nm-auth-utils.h"
-#include "nm-posix-signals.h"
 #include "nm-dbus-glib-types.h"
 
 /*
@@ -167,7 +166,7 @@ nm_spawn_process (const char *args)
 		return -1;
 	}
 
-	if (!g_spawn_sync ("/", argv, NULL, 0, nm_unblock_posix_signals, NULL, NULL, NULL, &status, &error)) {
+	if (!g_spawn_sync ("/", argv, NULL, 0, NULL, NULL, NULL, NULL, &status, &error)) {
 		nm_log_warn (LOGD_CORE, "could not spawn process '%s': %s", args, error->message);
 		g_error_free (error);
 	}

src/devices/nm-device.c

@@ -62,7 +62,6 @@
 #include "nm-enum-types.h"
 #include "nm-settings-connection.h"
 #include "nm-connection-provider.h"
-#include "nm-posix-signals.h"
 #include "nm-auth-utils.h"
 #include "nm-dbus-glib-types.h"
 #include "nm-dispatcher.h"
@@ -2726,12 +2725,6 @@ aipd_child_setup (gpointer user_data G_GNUC_UNUSED)
 	 */
 	pid_t pid = getpid ();
 	setpgid (pid, pid);
-
-	/*
-	 * We blocked signals in main(). We need to restore original signal
-	 * mask for avahi-autoipd here so that it can receive signals.
-	 */
-	nm_unblock_posix_signals (NULL);
 }
 
 /* default to installed helper, but can be modified for testing */
@@ -4955,8 +4948,6 @@ share_child_setup (gpointer user_data G_GNUC_UNUSED)
 	/* We are in the child process at this point */
 	pid_t pid = getpid ();
 	setpgid (pid, pid);
-
-	nm_unblock_posix_signals (NULL);
 }
 
 static gboolean
@@ -5113,8 +5104,7 @@ send_arps (NMDevice *self, const char *mode_arg)
 		       "arping: run %s", (tmp_str = g_strjoinv (" ", (char **) argv)));
 		success = g_spawn_async (NULL, (char **) argv, NULL,
 		                         G_SPAWN_STDOUT_TO_DEV_NULL | G_SPAWN_STDERR_TO_DEV_NULL,
-		                         nm_unblock_posix_signals,
+		                         NULL, NULL, NULL, &error);
-		                         NULL, NULL, &error);
 		if (!success) {
 			_LOGW (LOGD_DEVICE | LOGD_IP4,
 			       "arping: could not send ARP for local address %s: %s",
@@ -6149,7 +6139,7 @@ spawn_ping (NMDevice *self,
 	                         (gchar **) args,
 	                         NULL,
 	                         G_SPAWN_DO_NOT_REAP_CHILD,
-	                         nm_unblock_posix_signals,
+	                         NULL,
 	                         NULL,
 	                         &priv->gw_ping.pid,
 	                         &error);

src/devices/team/nm-device-team.c

@@ -39,7 +39,6 @@
 #include "nm-dbus-manager.h"
 #include "nm-enum-types.h"
 #include "nm-team-enum-types.h"
-#include "nm-posix-signals.h"
 #include "nm-core-internal.h"
 
 #include "nm-device-team-glue.h"
@@ -392,12 +391,6 @@ teamd_child_setup (gpointer user_data G_GNUC_UNUSED)
 	 */
 	pid_t pid = getpid ();
 	setpgid (pid, pid);
-
-	/*
-	 * We blocked signals in main(). We need to restore original signal
-	 * mask for avahi-autoipd here so that it can receive signals.
-	 */
-	nm_unblock_posix_signals (NULL);
 }
 
 static void
@@ -461,7 +454,7 @@ teamd_start (NMDevice *device, NMSettingTeam *s_team)
 	       (tmp_str = g_strjoinv (" ", (gchar **) argv->pdata)));
 	g_clear_pointer (&tmp_str, g_free);
 
-	ret = g_spawn_sync ("/", (char **) argv->pdata, NULL, 0, nm_unblock_posix_signals, NULL, NULL, NULL, &status, &error);
+	ret = g_spawn_sync ("/", (char **) argv->pdata, NULL, 0, NULL, NULL, NULL, NULL, &status, &error);
 	g_ptr_array_free (argv, TRUE);
 
 	/* Start teamd now */

src/dhcp-manager/nm-dhcp-dhclient.c

@@ -41,7 +41,6 @@
 #include "nm-logging.h"
 #include "nm-dhcp-dhclient-utils.h"
 #include "nm-dhcp-manager.h"
-#include "nm-posix-signals.h"
 #include "NetworkManagerUtils.h"
 #include "nm-dhcp-listener.h"
 #include "gsystem-local-alloc.h"
@@ -307,12 +306,6 @@ dhclient_child_setup (gpointer user_data G_GNUC_UNUSED)
 	/* We are in the child process at this point */
 	pid_t pid = getpid ();
 	setpgid (pid, pid);
-
-	/*
-	 * We blocked signals in main(). We need to restore original signal
-	 * mask for dhclient here so that it can receive signals.
-	 */
-	nm_unblock_posix_signals (NULL);
 }
 
 static gboolean

src/dhcp-manager/nm-dhcp-dhcpcd.c

@@ -37,7 +37,6 @@
 #include "nm-dhcp-manager.h"
 #include "nm-utils.h"
 #include "nm-logging.h"
-#include "nm-posix-signals.h"
 #include "NetworkManagerUtils.h"
 #include "nm-dhcp-listener.h"
 
@@ -65,12 +64,6 @@ dhcpcd_child_setup (gpointer user_data G_GNUC_UNUSED)
 	/* We are in the child process at this point */
 	pid_t pid = getpid ();
 	setpgid (pid, pid);
-
-	/*
-	 * We blocked signals in main(). We need to restore original signal
-	 * mask for dhcpcd here so that it can receive signals.
-	 */
-	nm_unblock_posix_signals (NULL);
 }
 
 static gboolean

src/dns-manager/nm-dns-manager.c

@@ -42,7 +42,6 @@
 #include "nm-ip6-config.h"
 #include "nm-logging.h"
 #include "NetworkManagerUtils.h"
-#include "nm-posix-signals.h"
 #include "nm-config.h"
 
 #include "nm-dns-plugin.h"
@@ -229,12 +228,6 @@ netconfig_child_setup (gpointer user_data G_GNUC_UNUSED)
 {
 	pid_t pid = getpid ();
 	setpgid (pid, pid);
-
-	/*
-	 * We blocked signals in main(). We need to restore original signal
-	 * mask for netconfig here so that it can receive signals.
-	 */
-	nm_unblock_posix_signals (NULL);
 }
 
 static GPid

src/dns-manager/nm-dns-plugin.c

@@ -28,7 +28,6 @@
 
 #include "nm-dns-plugin.h"
 #include "nm-logging.h"
-#include "nm-posix-signals.h"
 #include "NetworkManagerUtils.h"
 
 typedef struct {
@@ -142,12 +141,6 @@ child_setup (gpointer user_data G_GNUC_UNUSED)
 	/* We are in the child process at this point */
 	pid_t pid = getpid ();
 	setpgid (pid, pid);
-
-	/*
-	 * We blocked signals in main(). We need to restore original signal
-	 * mask for DNS plugin here so that it can receive signals.
-	 */
-	nm_unblock_posix_signals (NULL);
 }
 
 GPid

src/dnsmasq-manager/nm-dnsmasq-manager.c

@@ -32,7 +32,6 @@
 #include "nm-dnsmasq-utils.h"
 #include "nm-logging.h"
 #include "nm-glib-compat.h"
-#include "nm-posix-signals.h"
 #include "nm-utils.h"
 #include "NetworkManagerUtils.h"
 
@@ -302,12 +301,6 @@ dm_child_setup (gpointer user_data G_GNUC_UNUSED)
 	/* We are in the child process at this point */
 	pid_t pid = getpid ();
 	setpgid (pid, pid);
-
-	/*
-	 * We blocked signals in main(). We need to restore original signal
-	 * mask for dnsmasq here so that it can receive signals.
-	 */
-	nm_unblock_posix_signals (NULL);
 }
 
 static void

src/main-utils.c

@@ -31,101 +31,59 @@
 
 #include <glib.h>
 #include <glib/gi18n.h>
+#include <glib-unix.h>
 #include <gmodule.h>
 
 #include "main-utils.h"
-#include "nm-posix-signals.h"
 #include "nm-logging.h"
 
-static sigset_t signal_set;
+static gboolean
-static gboolean *quit_early = NULL;
+sighup_handler (gpointer user_data)
-
-/*
- * Thread function waiting for signals and processing them.
- * Wait for signals in signal set. The semantics of sigwait() require that all
- * threads (including the thread calling sigwait()) have the signal masked, for
- * reliable operation. Otherwise, a signal that arrives while this thread is
- * not blocked in sigwait() might be delivered to another thread.
- */
-static void *
-signal_handling_thread (void *arg)
 {
-	GMainLoop *main_loop = arg;
+	/* Reread config stuff like system config files, VPN service files, etc */
-	int signo;
+	nm_log_info (LOGD_CORE, "caught SIGHUP, not supported yet.");
 
-	while (1) {
+	return G_SOURCE_CONTINUE;
-		sigwait (&signal_set, &signo);
+}
 
-		switch (signo) {
+static gboolean
-		case SIGINT:
+sigint_handler (gpointer user_data)
-		case SIGTERM:
+{
-			nm_log_info (LOGD_CORE, "caught signal %d, shutting down normally.", signo);
+	GMainLoop *main_loop = user_data;
-			*quit_early = TRUE; /* for quitting before entering the main loop */
+
-			g_main_loop_quit (main_loop);
+	nm_log_info (LOGD_CORE, "caught SIGINT, shutting down normally.");
-			break;
+	g_main_loop_quit (main_loop);
-		case SIGHUP:
+
-			/* Reread config stuff like system config files, VPN service files, etc */
+	return G_SOURCE_REMOVE;
-			nm_log_info (LOGD_CORE, "caught signal %d, not supported yet.", signo);
+}
-			break;
+
-		case SIGPIPE:
+static gboolean
-			/* silently ignore signal */
+sigterm_handler (gpointer user_data)
-			break;
+{
-		default:
+	GMainLoop *main_loop = user_data;
-			nm_log_err (LOGD_CORE, "caught unexpected signal %d", signo);
+
-			break;
+	nm_log_info (LOGD_CORE, "caught SIGTERM, shutting down normally.");
-		}
+	g_main_loop_quit (main_loop);
-    }
+
-    return NULL;
+	return G_SOURCE_REMOVE;
 }
 
 /**
  * nm_main_utils_setup_signals:
  * @main_loop: the #GMainLoop to quit when SIGINT or SIGTERM is received
- * @quit_early: location of a variable that will be set to TRUE when
- *   SIGINT or SIGTERM is received
  *
- * Mask the signals we are interested in and create a signal handling thread.
+ * Sets up signal handling for NetworkManager.
- * Because all threads inherit the signal mask from their creator, all threads
- * in the process will have the signals masked. That's why setup_signals() has
- * to be called before creating other threads.
- *
- * Returns: %TRUE on success
  */
-gboolean
+void
-nm_main_utils_setup_signals (GMainLoop *main_loop, gboolean *quit_early_ptr)
+nm_main_utils_setup_signals (GMainLoop *main_loop)
 {
-	pthread_t signal_thread_id;
+	g_return_if_fail (main_loop != NULL);
-	sigset_t old_sig_mask;
-	int status;
 
-	g_return_val_if_fail (main_loop != NULL, FALSE);
+	signal (SIGPIPE, SIG_IGN);
-	g_return_val_if_fail (quit_early_ptr != NULL, FALSE);
 
-	quit_early = quit_early_ptr;
+	g_unix_signal_add (SIGHUP, sighup_handler, NULL);
-
+	g_unix_signal_add (SIGINT, sigint_handler, main_loop);
-	sigemptyset (&signal_set);
+	g_unix_signal_add (SIGTERM, sigterm_handler, main_loop);
-	sigaddset (&signal_set, SIGHUP);
-	sigaddset (&signal_set, SIGINT);
-	sigaddset (&signal_set, SIGTERM);
-	sigaddset (&signal_set, SIGPIPE);
-
-	/* Block all signals of interest. */
-	status = pthread_sigmask (SIG_BLOCK, &signal_set, &old_sig_mask);
-	if (status != 0) {
-		fprintf (stderr, _("Failed to set signal mask: %d"), status);
-		return FALSE;
-	}
-	/* Save original mask so that we could use it for child processes. */
-	nm_save_original_signal_mask (old_sig_mask);
-
-	/* Create the signal handling thread. */
-	status = pthread_create (&signal_thread_id, NULL, signal_handling_thread, main_loop);
-	if (status != 0) {
-		fprintf (stderr, _("Failed to create signal handling thread: %d"), status);
-		return FALSE;
-	}
-
-	return TRUE;
 }
 
 gboolean

src/main-utils.h

@@ -23,7 +23,7 @@
 
 #include <glib.h>
 
-gboolean nm_main_utils_setup_signals (GMainLoop *main_loop, gboolean *quit_early_ptr);
+void     nm_main_utils_setup_signals (GMainLoop *main_loop);
 
 gboolean nm_main_utils_write_pidfile (const char *pidfile);
 

src/main.c

@@ -51,7 +51,6 @@
 #include "nm-dhcp-manager.h"
 #include "nm-logging.h"
 #include "nm-config.h"
-#include "nm-posix-signals.h"
 #include "nm-session-monitor.h"
 #include "nm-dispatcher.h"
 #include "nm-settings.h"
@@ -205,7 +204,6 @@ main (int argc, char *argv[])
 	GError *error = NULL;
 	gboolean wrote_pidfile = FALSE;
 	char *bad_domains = NULL;
-	gboolean quit_early = FALSE;
 
 	GOptionEntry options[] = {
 		{ "version", 'V', 0, G_OPTION_ARG_NONE, &show_version, N_("Print NetworkManager version and exit"), NULL },
@@ -345,8 +343,7 @@ main (int argc, char *argv[])
 	_init_nm_debug (nm_config_get_debug (config));
 
 	/* Set up unix signal handling - before creating threads, but after daemonizing! */
-	if (!nm_main_utils_setup_signals (main_loop, &quit_early))
+	nm_main_utils_setup_signals (main_loop);
-		exit (1);
 
 	if (g_fatal_warnings) {
 		GLogLevelFlags fatal_mask;
@@ -442,9 +439,7 @@ main (int argc, char *argv[])
 
 	success = TRUE;
 
-	/* Told to quit before getting to the mainloop by the signal handler */
+	g_main_loop_run (main_loop);
-	if (!quit_early)
-		g_main_loop_run (main_loop);
 
 	nm_manager_stop (manager);
 

src/nm-activation-request.c

@@ -35,7 +35,6 @@
 #include "nm-device.h"
 #include "nm-active-connection.h"
 #include "nm-settings-connection.h"
-#include "nm-posix-signals.h"
 #include "nm-auth-subject.h"
 
 G_DEFINE_TYPE (NMActRequest, nm_act_request, NM_TYPE_ACTIVE_CONNECTION)
@@ -202,8 +201,6 @@ share_child_setup (gpointer user_data G_GNUC_UNUSED)
 	/* We are in the child process at this point */
 	pid_t pid = getpid ();
 	setpgid (pid, pid);
-
-	nm_unblock_posix_signals (NULL);
 }
 
 void

src/nm-dcb.c

@@ -27,7 +27,6 @@
 #include "nm-dcb.h"
 #include "nm-platform.h"
 #include "NetworkManagerUtils.h"
-#include "nm-posix-signals.h"
 #include "nm-logging.h"
 
 static const char *helper_names[] = { "dcbtool", "fcoeadm" };
@@ -305,7 +304,7 @@ run_helper (char **argv, guint which, gpointer user_data, GError **error)
 	nm_log_dbg (LOGD_DCB, "%s", cmdline);
 
 	success = g_spawn_sync ("/", argv, NULL, 0 /*G_SPAWN_DEFAULT*/,
-	                        nm_unblock_posix_signals, NULL,
+	                        NULL, NULL,
 	                        &outmsg, &errmsg, &exit_status, error);
 	/* Log any stderr output */
 	if (success && WIFEXITED (exit_status) && WEXITSTATUS (exit_status) && (errmsg || outmsg)) {

src/nm-posix-signals.c

@@ -1,62 +0,0 @@
-/* -*- Mode: C; tab-width: 4; indent-tabs-mode: t; c-basic-offset: 4 -*- */
-/* NetworkManager -- Network link manager
- *
- * This program is free software; you can redistribute it and/or modify
- * it under the terms of the GNU General Public License as published by
- * the Free Software Foundation; either version 2 of the License, or
- * (at your option) any later version.
- *
- * This program is distributed in the hope that it will be useful,
- * but WITHOUT ANY WARRANTY; without even the implied warranty of
- * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
- * GNU General Public License for more details.
- *
- * You should have received a copy of the GNU General Public License along
- * with this program; if not, write to the Free Software Foundation, Inc.,
- * 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA.
- *
- * Copyright (C) 2012 Red Hat, Inc.
- */
-
-#include "config.h"
-
-#include <signal.h>
-
-#include "nm-posix-signals.h"
-
-
-/* Stores the original signal mask of NetworkManager process */
-static sigset_t nm_original_signal_mask;
-
-void
-nm_save_original_signal_mask (sigset_t sig_mask)
-{
-	nm_original_signal_mask = sig_mask;
-}
-
-const sigset_t *
-nm_get_original_signal_mask (void)
-{
-	return &nm_original_signal_mask;
-}
-
-/* 
- * Unblock signals.
- * If a signal set is passed, those signals are unblocked. If user_data is NULL
- * the process' signal mask is set to the saved original mask.
- * Note: This function can be used in g_spawn_* as GSpawnChildSetupFunc()
- *       callback.
- */
-void
-nm_unblock_posix_signals (gpointer user_data)
-{
-	sigset_t *user_sigset = (sigset_t *) user_data;
-
-	if (user_sigset != NULL) {
-		pthread_sigmask (SIG_UNBLOCK, user_sigset, NULL);
-	} else {
-		const sigset_t *orig_sig_mask = nm_get_original_signal_mask ();
-		pthread_sigmask (SIG_SETMASK, orig_sig_mask, NULL);
-	}
-}
-

src/nm-posix-signals.h

@@ -1,36 +0,0 @@
-/* -*- Mode: C; tab-width: 4; indent-tabs-mode: t; c-basic-offset: 4 -*- */
-/* NetworkManager -- Network link manager
- *
- * This program is free software; you can redistribute it and/or modify
- * it under the terms of the GNU General Public License as published by
- * the Free Software Foundation; either version 2 of the License, or
- * (at your option) any later version.
- *
- * This program is distributed in the hope that it will be useful,
- * but WITHOUT ANY WARRANTY; without even the implied warranty of
- * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
- * GNU General Public License for more details.
- *
- * You should have received a copy of the GNU General Public License along
- * with this program; if not, write to the Free Software Foundation, Inc.,
- * 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA.
- *
- * Copyright (C) 2012 Red Hat, Inc.
- */
-
-#ifndef __NETWORKMANAGER_POSIX_SIGNALS_H__
-#define __NETWORKMANAGER_POSIX_SIGNALS_H__
-
-#include <glib.h>
-#include <signal.h>
-
-/*
- * This function can be used in g_spawn_* as GSpawnChildSetupFunc()
- * callback.
- */
-void nm_unblock_posix_signals (gpointer user_data);
-
-void nm_save_original_signal_mask (sigset_t sig_mask);
-const sigset_t *nm_get_original_signal_mask (void);
-
-#endif /* __NETWORKMANAGER_POSIX_SIGNALS_H__ */

src/ppp-manager/nm-ppp-manager.c

@@ -46,7 +46,6 @@
 #include "nm-ppp-manager.h"
 #include "nm-dbus-manager.h"
 #include "nm-logging.h"
-#include "nm-posix-signals.h"
 #include "nm-platform.h"
 #include "nm-core-internal.h"
 
@@ -1032,12 +1031,6 @@ pppd_child_setup (gpointer user_data G_GNUC_UNUSED)
 	/* We are in the child process at this point */
 	pid_t pid = getpid ();
 	setpgid (pid, pid);
-
-	/*
-	 * We blocked signals in main(). We need to restore original signal
-	 * mask for pppd here so that it can receive signals.
-	 */
-	nm_unblock_posix_signals (NULL);
 }
 
 static void

src/settings/plugins/ibft/reader.c

@@ -36,7 +36,6 @@
 
 #include "nm-core-internal.h"
 #include "nm-platform.h"
-#include "nm-posix-signals.h"
 #include "NetworkManagerUtils.h"
 #include "nm-logging.h"
 
@@ -52,12 +51,6 @@ iscsiadm_child_setup (gpointer user_data G_GNUC_UNUSED)
 	 */
 	pid_t pid = getpid ();
 	setpgid (pid, pid);
-
-	/*
-	 * We blocked signals in main(). We need to restore original signal
-	 * mask for iscsiadm here so that it can receive signals.
-	 */
-	nm_unblock_posix_signals (NULL);
 }
 
 /* Removes trailing whitespace and whitespace before and immediately after the '=' */

src/settings/plugins/ifcfg-rh/reader.c

@@ -53,7 +53,6 @@
 #include <nm-utils.h>
 
 #include "nm-platform.h"
-#include "nm-posix-signals.h"
 #include "NetworkManagerUtils.h"
 #include "nm-logging.h"
 

src/vpn-manager/nm-vpn-service.c

@@ -31,7 +31,6 @@
 #include "nm-vpn-service.h"
 #include "nm-dbus-manager.h"
 #include "nm-logging.h"
-#include "nm-posix-signals.h"
 #include "nm-vpn-manager.h"
 #include "nm-glib-compat.h"
 
@@ -173,12 +172,6 @@ _daemon_setup (gpointer user_data G_GNUC_UNUSED)
 	/* We are in the child process at this point */
 	pid_t pid = getpid ();
 	setpgid (pid, pid);
-
-	/*
-	 * We blocked signals in main(). We need to restore original signal
-	 * mask for VPN service here so that it can receive signals.
-	 */
-	nm_unblock_posix_signals (NULL);
 }
 
 static gboolean