fatal-signal: Run signal hooks outside of actual signal handlers.
authorJesse Gross <jesse@nicira.com>
Tue, 8 Dec 2009 22:11:22 +0000 (14:11 -0800)
committerJesse Gross <jesse@nicira.com>
Wed, 6 Jan 2010 14:11:58 +0000 (09:11 -0500)
Rather than running signal hooks directly from the actual signal
handler, simply record the fact that the signal occured and run
the hook next time around the poll loop.  This allows significantly
more freedom as to what can actually be done in the signal hooks.

lib/fatal-signal.c
lib/fatal-signal.h
lib/netdev.c
lib/poll-loop.c
lib/process.c
tests/test-dhcp-client.c
utilities/ovs-discover.c

index 8180521..64b045d 100644 (file)
 #include <signal.h>
 #include <stdbool.h>
 #include <stdio.h>
+#include <stdint.h>
 #include <stdlib.h>
 #include <string.h>
 #include <unistd.h>
+#include "poll-loop.h"
 #include "shash.h"
+#include "socket-util.h"
 #include "util.h"
 
 #define THIS_MODULE VLM_fatal_signal
@@ -45,53 +48,32 @@ struct hook {
 static struct hook hooks[MAX_HOOKS];
 static size_t n_hooks;
 
-/* Number of nesting signal blockers. */
-static int block_level = 0;
-
-/* Signal mask saved by outermost signal blocker. */
-static sigset_t saved_signal_mask;
+static int signal_fds[2];
+static volatile sig_atomic_t stored_sig_nr = SIG_ATOMIC_MAX;
 
 /* Disabled by fatal_signal_fork()? */
 static bool disabled;
 
-static void call_sigprocmask(int how, sigset_t* new_set, sigset_t* old_set);
+static void fatal_signal_init(void);
 static void atexit_handler(void);
 static void call_hooks(int sig_nr);
 
-/* Registers 'hook' to be called when a process termination signal is raised.
- * If 'run_at_exit' is true, 'hook' is also called during normal process
- * termination, e.g. when exit() is called or when main() returns.
- *
- * 'func' will be invoked from an asynchronous signal handler, so it must be
- * written appropriately.  For example, it must not call most C library
- * functions, including malloc() or free(). */
-void
-fatal_signal_add_hook(void (*func)(void *aux), void *aux, bool run_at_exit)
-{
-    fatal_signal_block();
-    assert(n_hooks < MAX_HOOKS);
-    hooks[n_hooks].func = func;
-    hooks[n_hooks].aux = aux;
-    hooks[n_hooks].run_at_exit = run_at_exit;
-    n_hooks++;
-    fatal_signal_unblock();
-}
-
-/* Blocks program termination signals until fatal_signal_unblock() is called.
- * May be called multiple times with nesting; if so, fatal_signal_unblock()
- * must be called the same number of times to unblock signals.
- *
- * This is needed while adjusting a data structure that will be accessed by a
- * fatal signal hook, so that the hook is not invoked while the data structure
- * is in an inconsistent state. */
-void
-fatal_signal_block(void)
+static void
+fatal_signal_init(void)
 {
     static bool inited = false;
+
     if (!inited) {
         size_t i;
 
         inited = true;
+
+        if (pipe(signal_fds)) {
+            ovs_fatal(errno, "could not create pipe");
+        }
+        set_nonblocking(signal_fds[0]);
+        set_nonblocking(signal_fds[1]);
+
         sigemptyset(&fatal_signal_set);
         for (i = 0; i < ARRAY_SIZE(fatal_signals); i++) {
             int sig_nr = fatal_signals[i];
@@ -108,23 +90,24 @@ fatal_signal_block(void)
         }
         atexit(atexit_handler);
     }
-
-    if (++block_level == 1) {
-        call_sigprocmask(SIG_BLOCK, &fatal_signal_set, &saved_signal_mask);
-    }
 }
 
-/* Unblocks program termination signals blocked by fatal_signal_block() is
- * called.  If multiple calls to fatal_signal_block() are nested,
- * fatal_signal_unblock() must be called the same number of times to unblock
- * signals. */
+/* Registers 'hook' to be called when a process termination signal is raised.
+ * If 'run_at_exit' is true, 'hook' is also called during normal process
+ * termination, e.g. when exit() is called or when main() returns.
+ *
+ * The hook is not called immediately from the signal handler but rather the
+ * next time the poll loop iterates, so it is freed from the usual restrictions
+ * on signal handler functions. */
 void
-fatal_signal_unblock(void)
+fatal_signal_add_hook(void (*func)(void *aux), void *aux, bool run_at_exit)
 {
-    assert(block_level > 0);
-    if (--block_level == 0) {
-        call_sigprocmask(SIG_SETMASK, &saved_signal_mask, NULL);
-    }
+    fatal_signal_init();
+    assert(n_hooks < MAX_HOOKS);
+    hooks[n_hooks].func = func;
+    hooks[n_hooks].aux = aux;
+    hooks[n_hooks].run_at_exit = run_at_exit;
+    n_hooks++;
 }
 
 /* Handles fatal signal number 'sig_nr'.
@@ -139,12 +122,29 @@ fatal_signal_unblock(void)
 void
 fatal_signal_handler(int sig_nr)
 {
-    call_hooks(sig_nr);
+    ignore(write(signal_fds[1], "", 1));
+    stored_sig_nr = sig_nr;
+}
+
+void
+fatal_signal_run(void)
+{
+    int sig_nr = stored_sig_nr;
 
-    /* Re-raise the signal with the default handling so that the program
-     * termination status reflects that we were killed by this signal */
-    signal(sig_nr, SIG_DFL);
-    raise(sig_nr);
+    if (sig_nr != SIG_ATOMIC_MAX) {
+        call_hooks(sig_nr);
+
+        /* Re-raise the signal with the default handling so that the program
+         * termination status reflects that we were killed by this signal */
+        signal(sig_nr, SIG_DFL);
+        raise(sig_nr);
+    }
+}
+
+void
+fatal_signal_wait(void)
+{
+    poll_fd_wait(signal_fds[0], POLLIN);
 }
 
 static void
@@ -189,11 +189,9 @@ fatal_signal_add_file_to_unlink(const char *file)
         fatal_signal_add_hook(unlink_files, NULL, true);
     }
 
-    fatal_signal_block();
     if (!shash_find(&files, file)) {
         shash_add(&files, file, NULL);
     }
-    fatal_signal_unblock();
 }
 
 /* Unregisters 'file' from being unlinked when the program terminates via
@@ -203,12 +201,10 @@ fatal_signal_remove_file_to_unlink(const char *file)
 {
     struct shash_node *node;
 
-    fatal_signal_block();
     node = shash_find(&files, file);
     if (node) {
         shash_delete(&files, node);
     }
-    fatal_signal_unblock();
 }
 
 /* Like fatal_signal_remove_file_to_unlink(), but also unlinks 'file'.
@@ -232,12 +228,6 @@ unlink_files(void *aux UNUSED)
     do_unlink_files(); 
 }
 
-/* This is a fatal_signal_add_hook() callback (via unlink_files()).  It will be
- * invoked from an asynchronous signal handler, so it cannot call most C
- * library functions (unlink() is an explicit exception, see
- * http://www.opengroup.org/onlinepubs/009695399/functions/xsh_chap02_04.html).
- * That includes free(), so it doesn't try to free the 'files' data
- * structure. */
 static void
 do_unlink_files(void)
 {
@@ -265,13 +255,10 @@ fatal_signal_fork(void)
             signal(sig_nr, SIG_IGN);
         }
     }
-}
-\f
-static void
-call_sigprocmask(int how, sigset_t* new_set, sigset_t* old_set)
-{
-    int error = sigprocmask(how, new_set, old_set);
-    if (error) {
-        fprintf(stderr, "sigprocmask: %s\n", strerror(errno));
+
+    /* Raise any signals that we have already received with the default
+     * handler. */
+    if (stored_sig_nr != SIG_ATOMIC_MAX) {
+        raise(stored_sig_nr);
     }
 }
index c96db86..a84d5fd 100644 (file)
@@ -21,9 +21,9 @@
 
 /* Basic interface. */
 void fatal_signal_add_hook(void (*)(void *aux), void *aux, bool run_at_exit);
-void fatal_signal_block(void);
-void fatal_signal_unblock(void);
 void fatal_signal_fork(void);
+void fatal_signal_run(void);
+void fatal_signal_wait(void);
 
 /* Convenience functions for unlinking files upon termination.
  *
index 6c87bd4..77964f9 100644 (file)
@@ -297,10 +297,8 @@ netdev_close(struct netdev *netdev)
 #endif
 
         /* Restore flags that we changed, if any. */
-        fatal_signal_block();
         error = restore_flags(netdev);
         list_remove(&netdev->node);
-        fatal_signal_unblock();
         if (error) {
             VLOG_WARN("failed to restore network device flags on %s: %s",
                       name, strerror(error));
index 32bbc13..81dc8fd 100644 (file)
@@ -25,6 +25,7 @@
 #include "backtrace.h"
 #include "coverage.h"
 #include "dynamic-string.h"
+#include "fatal-signal.h"
 #include "list.h"
 #include "timeval.h"
 
@@ -147,6 +148,10 @@ poll_block(void)
     int n_pollfds;
     int retval;
 
+    /* Register fatal signal events before actually doing any real work for
+     * poll_block. */
+    fatal_signal_wait();
+
     assert(!running_cb);
     if (max_pollfds < n_waiters) {
         max_pollfds = n_waiters;
@@ -207,6 +212,9 @@ poll_block(void)
 
     timeout = -1;
     timeout_backtrace.n_frames = 0;
+
+    /* Handle any pending signals before doing anything else. */
+    fatal_signal_run();
 }
 
 /* Registers 'function' to be called with argument 'aux' by poll_block() when
index 12168f7..3f66ddd 100644 (file)
@@ -201,17 +201,14 @@ process_start(char **argv,
     }
 
     block_sigchld(&oldsigs);
-    fatal_signal_block();
     pid = fork();
     if (pid < 0) {
-        fatal_signal_unblock();
         unblock_sigchld(&oldsigs);
         VLOG_WARN("fork failed: %s", strerror(errno));
         return errno;
     } else if (pid) {
         /* Running in parent process. */
         *pp = process_register(argv[0], pid);
-        fatal_signal_unblock();
         unblock_sigchld(&oldsigs);
         return 0;
     } else {
@@ -220,7 +217,6 @@ process_start(char **argv,
         int fd;
 
         fatal_signal_fork();
-        fatal_signal_unblock();
         unblock_sigchld(&oldsigs);
         for (fd = 0; fd < fd_max; fd++) {
             if (is_member(fd, null_fds, n_null_fds)) {
@@ -521,12 +517,10 @@ process_run_capture(char **argv, char **stdout_log, char **stderr_log,
     }
 
     block_sigchld(&oldsigs);
-    fatal_signal_block();
     pid = fork();
     if (pid < 0) {
         int error = errno;
 
-        fatal_signal_unblock();
         unblock_sigchld(&oldsigs);
         VLOG_WARN("fork failed: %s", strerror(error));
 
@@ -539,7 +533,6 @@ process_run_capture(char **argv, char **stdout_log, char **stderr_log,
         struct process *p;
 
         p = process_register(argv[0], pid);
-        fatal_signal_unblock();
         unblock_sigchld(&oldsigs);
 
         close(s_stdout.fds[1]);
@@ -575,7 +568,6 @@ process_run_capture(char **argv, char **stdout_log, char **stderr_log,
         int i;
 
         fatal_signal_fork();
-        fatal_signal_unblock();
         unblock_sigchld(&oldsigs);
 
         dup2(get_null_fd(), 0);
index e4471c7..c15f693 100644 (file)
@@ -70,7 +70,6 @@ main(int argc, char *argv[])
     fatal_signal_add_hook(release, cli, true);
 
     for (;;) {
-        fatal_signal_block();
         dhclient_run(cli);
         if (dhclient_changed(cli)) {
             dhclient_configure_netdev(cli);
@@ -79,7 +78,6 @@ main(int argc, char *argv[])
             }
         }
         dhclient_wait(cli);
-        fatal_signal_unblock();
         poll_block();
     }
 }
index dc91bce..3aa28fa 100644 (file)
@@ -122,7 +122,6 @@ main(int argc, char *argv[])
 
     signal(SIGPIPE, SIG_IGN);
     for (;;) {
-        fatal_signal_block();
         for (i = 0; i < n_ifaces; i++) {
             struct iface *iface = &ifaces[i];
             dhclient_run(iface->dhcp);
@@ -195,7 +194,6 @@ main(int argc, char *argv[])
             dhclient_wait(iface->dhcp);
         }
         unixctl_server_wait(unixctl);
-        fatal_signal_unblock();
         poll_block();
     }