Attachment 577696 Details for Bug 739836 – [PATCH] improve signal handling

[patch] [PATCH] improve signal handling

0001-core-improve-handling-of-POSIX-signals-using-sigwait.patch (text/plain), 7.25 KB, created by Jirka Klimes on 2012-04-16 12:22:52 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Jirka Klimes

Created: 2012-04-16 12:22:52 UTC

Size: 7.25 KB

patch

obsolete

>From acd0545685ed6e7f93e83602e9e23ee0fbb581b5 Mon Sep 17 00:00:00 2001
>From: =?UTF-8?q?Ji=C5=99=C3=AD=20Klime=C5=A1?= <jklimes@redhat.com>
>Date: Mon, 16 Apr 2012 13:30:53 +0200
>Subject: [PATCH] core: improve handling of POSIX signals using sigwait() (rh
> #739836)
>MIME-Version: 1.0
>Content-Type: text/plain; charset=UTF-8
>Content-Transfer-Encoding: 8bit
>
>There are multiple ways how to handle standard unix signals. They work quite
>well for a single-threaded application. However, signal handling in a multi-
>threaded app becomes tricky. And, the most safe way is to use sigwait() function
>in a dedicated thread, which allows us to process the signals synchronously and
>thus avoid various nasty problems.
>
>A few useful links:
>http://pubs.opengroup.org/onlinepubs/007904975/functions/sigwait.html
>http://pic.dhe.ibm.com/infocenter/aix/v6r1/index.jsp?topic=%2Fcom.ibm.aix.genprogc%2Fdoc%2Fgenprogc%2Fsignal_mgmt.htm
>http://www.linuxjournal.com/article/2121?page=0,2
>http://www.redwoodsoft.com/~dru/unixbook/book.chinaunix.net/special/ebook/addisonWesley/APUE2/0201433079/ch12lev1sec8.html
>
>Signed-off-by: JiÅÃ KlimeÅ¡ <jklimes@redhat.com>
>---
> src/main.c |  179 +++++++++++++++++++++++++++++++-----------------------------
> 1 files changed, 92 insertions(+), 87 deletions(-)
>
>diff --git a/src/main.c b/src/main.c
>index 4a21959..5eed908 100644
>--- a/src/main.c
>+++ b/src/main.c
>@@ -66,101 +66,104 @@
>  */
> static NMManager *manager = NULL;
> static GMainLoop *main_loop = NULL;
>-static int quit_pipe[2] = { -1, -1 };
>-
> static gboolean quit_early = FALSE;
>+static sigset_t signal_set;
> 
>-static void
>-nm_signal_handler (int signo)
>+void *signal_handling_thread (void *arg);
>+/*
>+ * 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.
>+ */
>+void *
>+signal_handling_thread (void *arg)
> {
>-	static int in_fatal = 0, x;
>-
>-	/* avoid loops */
>-	if (in_fatal > 0)
>-		return;
>-	++in_fatal;
>-
>-	switch (signo) {
>-	case SIGSEGV:
>-	case SIGBUS:
>-	case SIGILL:
>-	case SIGABRT:
>-		nm_log_warn (LOGD_CORE, "caught signal %d. Generating backtrace...", signo);
>-		nm_logging_backtrace ();
>-		exit (1);
>-		break;
>-	case SIGFPE:
>-	case SIGPIPE:
>-		/* let the fatal signals interrupt us */
>-		--in_fatal;
>-		nm_log_warn (LOGD_CORE, "caught signal %d, shutting down abnormally. Generating backtrace...", signo);
>-		nm_logging_backtrace ();
>-		x = write (quit_pipe[1], "X", 1);
>-		break;
>-	case SIGINT:
>-	case SIGTERM:
>-		/* let the fatal signals interrupt us */
>-		--in_fatal;
>-		nm_log_info (LOGD_CORE, "caught signal %d, shutting down normally.", signo);
>-		quit_early = TRUE;
>-		x = write (quit_pipe[1], "X", 1);
>-		break;
>-	case SIGHUP:
>-		--in_fatal;
>-		/* Reread config stuff like system config files, VPN service files, etc */
>-		break;
>-	case SIGUSR1:
>-		--in_fatal;
>-		/* Play with log levels or something */
>-		break;
>-	default:
>-		signal (signo, nm_signal_handler);
>-		break;
>-	}
>+	int signo;
>+
>+	while (1) {
>+		sigwait (&signal_set, &signo);
>+
>+		switch (signo) {
>+		case SIGSEGV:
>+		case SIGBUS:
>+		case SIGILL:
>+		case SIGABRT:
>+		case SIGQUIT:
>+			nm_log_warn (LOGD_CORE, "caught signal %d. Generating backtrace...", signo);
>+			nm_logging_backtrace ();
>+			exit (1);
>+			break;
>+		case SIGFPE:
>+		case SIGPIPE:
>+			nm_log_warn (LOGD_CORE, "caught signal %d, shutting down abnormally. Generating backtrace...", signo);
>+			nm_logging_backtrace ();
>+			quit_early = TRUE; /* for quitting before entering the main loop */
>+			g_main_loop_quit (main_loop);
>+			break;
>+		case SIGINT:
>+		case SIGTERM:
>+			nm_log_info (LOGD_CORE, "caught signal %d, shutting down normally.", signo);
>+			quit_early = TRUE; /* for quitting before entering the main loop */
>+			g_main_loop_quit (main_loop);
>+			break;
>+		case SIGHUP:
>+			/* Reread config stuff like system config files, VPN service files, etc */
>+			nm_log_info (LOGD_CORE, "caught signal %d, not supported yet.", signo);
>+			break;
>+		case SIGUSR1:
>+			/* Play with log levels or something */
>+			nm_log_info (LOGD_CORE, "caught signal %d, not supported yet.", signo);
>+			break;
>+		default:
>+			nm_log_err (LOGD_CORE, "caught unexpected signal %d", signo);
>+			break;
>+		}
>+    }
>+    return NULL;
> }
> 
>+/*
>+ * Mask the signals we are interested in and create a signal handling thread.
>+ * 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.
>+ */
> static gboolean
>-quit_watch (GIOChannel *src, GIOCondition condition, gpointer user_data)
>-{
>-
>-	if (condition & G_IO_IN) {
>-		nm_log_warn (LOGD_CORE, "quit request received, terminating...");
>-		g_main_loop_quit (main_loop);
>-	}
>-
>-	return FALSE;
>-}
>-
>-static void
> setup_signals (void)
> {
>-	struct sigaction action;
>-	sigset_t mask;
>-	GIOChannel *quit_channel;
>+	pthread_t signal_thread_id;
>+	int status;
>+
>+	sigemptyset (&signal_set);
>+	sigaddset (&signal_set, SIGHUP);
>+	sigaddset (&signal_set, SIGINT);
>+	sigaddset (&signal_set, SIGQUIT);
>+	sigaddset (&signal_set, SIGILL);
>+	sigaddset (&signal_set, SIGABRT);
>+	sigaddset (&signal_set, SIGFPE);
>+	sigaddset (&signal_set, SIGBUS);
>+	sigaddset (&signal_set, SIGSEGV);
>+	sigaddset (&signal_set, SIGPIPE);
>+	sigaddset (&signal_set, SIGTERM);
>+	sigaddset (&signal_set, SIGUSR1);
>+
>+	/* Block all signals of interest. */
>+	status = pthread_sigmask (SIG_BLOCK, &signal_set, NULL);
>+	if (status != 0) {
>+		fprintf (stderr, _("Failed to set signal mask: %d"), status);
>+		return FALSE;
>+	}
> 
>-	/* Set up our quit pipe */
>-	if (pipe (quit_pipe) < 0) {
>-		fprintf (stderr, _("Failed to initialize SIGTERM pipe: %d"), errno);
>-		exit (1);
>+	/* Create the signal handling thread. */
>+	status = pthread_create (&signal_thread_id, NULL, signal_handling_thread, NULL);
>+	if (status != 0) {
>+		fprintf (stderr, _("Failed to create signal handling thread: %d"), status);
>+		return FALSE;
> 	}
>-	fcntl (quit_pipe[1], F_SETFL, O_NONBLOCK | fcntl (quit_pipe[1], F_GETFL));
>-
>-	quit_channel = g_io_channel_unix_new (quit_pipe[0]);
>-	g_io_add_watch_full (quit_channel, G_PRIORITY_HIGH, G_IO_IN | G_IO_ERR, quit_watch, NULL, NULL);
>-
>-	sigemptyset (&mask);
>-	action.sa_handler = nm_signal_handler;
>-	action.sa_mask = mask;
>-	action.sa_flags = 0;
>-	sigaction (SIGTERM,  &action, NULL);
>-	sigaction (SIGINT,  &action, NULL);
>-	sigaction (SIGILL,  &action, NULL);
>-	sigaction (SIGBUS,  &action, NULL);
>-	sigaction (SIGFPE,  &action, NULL);
>-	sigaction (SIGHUP,  &action, NULL);
>-	sigaction (SIGSEGV, &action, NULL);
>-	sigaction (SIGABRT, &action, NULL);
>-	sigaction (SIGUSR1,  &action, NULL);
>+
>+	return TRUE;
> }
> 
> static gboolean
>@@ -393,6 +396,10 @@ main (int argc, char *argv[])
> 		exit (1);
> 	}
> 
>+	/* Set up unix signal handling */
>+	if (!setup_signals ())
>+		exit (1);
>+
> 	/* Set locale to be able to use environment variables */
> 	setlocale (LC_ALL, "");
> 
>@@ -529,8 +536,6 @@ main (int argc, char *argv[])
> 	dbus_glib_global_set_disable_legacy_property_access ();
> #endif
> 
>-	setup_signals ();
>-
> 	nm_logging_start (become_daemon);
> 
> 	nm_log_info (LOGD_CORE, "NetworkManager (version " NM_DIST_VERSION ") is starting...");
>-- 
>1.7.7.6
>

Actions: View | Diff

Attachments on bug 739836: 523968 | 524361 | 533610 | 555210 | 555211 | 575228 | 575571 | 577696 | 577796