From: Jesse Gross Date: Tue, 8 Dec 2009 22:11:22 +0000 (-0800) Subject: fatal-signal: Run signal hooks outside of actual signal handlers. X-Git-Tag: v1.0.0~259^2~345 X-Git-Url: http://git.onelab.eu/?a=commitdiff_plain;h=d8b30702057c18dac2f35fd766ef5d2a12786eae;p=sliver-openvswitch.git fatal-signal: Run signal hooks outside of actual signal handlers. 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. --- diff --git a/lib/fatal-signal.c b/lib/fatal-signal.c index 818052143..64b045d37 100644 --- a/lib/fatal-signal.c +++ b/lib/fatal-signal.c @@ -20,10 +20,13 @@ #include #include #include +#include #include #include #include +#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); } } -} - -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); } } diff --git a/lib/fatal-signal.h b/lib/fatal-signal.h index c96db86dd..a84d5fd71 100644 --- a/lib/fatal-signal.h +++ b/lib/fatal-signal.h @@ -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. * diff --git a/lib/netdev.c b/lib/netdev.c index 6c87bd4b3..77964f93d 100644 --- a/lib/netdev.c +++ b/lib/netdev.c @@ -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)); diff --git a/lib/poll-loop.c b/lib/poll-loop.c index 32bbc1349..81dc8fdd8 100644 --- a/lib/poll-loop.c +++ b/lib/poll-loop.c @@ -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 diff --git a/lib/process.c b/lib/process.c index 12168f7e2..3f66ddd91 100644 --- a/lib/process.c +++ b/lib/process.c @@ -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); diff --git a/tests/test-dhcp-client.c b/tests/test-dhcp-client.c index e4471c7b2..c15f6939a 100644 --- a/tests/test-dhcp-client.c +++ b/tests/test-dhcp-client.c @@ -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(); } } diff --git a/utilities/ovs-discover.c b/utilities/ovs-discover.c index dc91bce3d..3aa28fad8 100644 --- a/utilities/ovs-discover.c +++ b/utilities/ovs-discover.c @@ -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(); }