From e3830e909d210145cbe52a1da66058ebf2581fd6 Mon Sep 17 00:00:00 2001 From: Ben Pfaff Date: Fri, 15 Jan 2010 15:28:14 -0800 Subject: [PATCH] fatal-signal: After fork, clear hooks instead of disabling them. Until now, fatal_signal_fork() has simply disabled all the fatal signal callback hooks. This worked fine, because a daemon process forked only once and the parent didn't do much before it exited. But upcoming commits will introduce a --monitor option, which requires processes to fork multiple times. Sometimes the parent process will fork, then run for a while, then fork again. It's not good to disable the hooks in the child process in such a case, because that prevents e.g. pidfiles from being removed at the child's exit. So this commit changes the semantics of fatal_signal_fork() to just clearing out hooks. After hooks are cleared, new hooks can be added and will be executed on process termination in the usual way. This commit also introduces a cancellation callback function so that a canceled hook can free resources. --- extras/ezio/ovs-switchui.c | 4 +- extras/ezio/tty.c | 4 +- lib/fatal-signal.c | 77 ++++++++++++++++++++++++-------------- lib/fatal-signal.h | 6 ++- lib/netdev.c | 3 +- tests/test-dhcp-client.c | 4 +- utilities/ovs-discover.c | 2 +- 7 files changed, 62 insertions(+), 38 deletions(-) diff --git a/extras/ezio/ovs-switchui.c b/extras/ezio/ovs-switchui.c index 365a708b..864e34c1 100644 --- a/extras/ezio/ovs-switchui.c +++ b/extras/ezio/ovs-switchui.c @@ -1,4 +1,4 @@ -/* Copyright (c) 2008, 2009 Nicira Networks, Inc. +/* Copyright (c) 2008, 2009, 2010 Nicira Networks, Inc. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -165,7 +165,7 @@ main(int argc, char *argv[]) daemonize(); initialize_terminal(); - fatal_signal_add_hook(restore_terminal, NULL, true); + fatal_signal_add_hook(restore_terminal, NULL, NULL, true); msg = NULL; countdown = 0; diff --git a/extras/ezio/tty.c b/extras/ezio/tty.c index 709b8028..9b05480a 100644 --- a/extras/ezio/tty.c +++ b/extras/ezio/tty.c @@ -1,4 +1,4 @@ -/* Copyright (c) 2008, 2009 Nicira Networks, Inc. +/* Copyright (c) 2008, 2009, 2010 Nicira Networks, Inc. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -263,7 +263,7 @@ tty_set_raw_mode(int fd, speed_t speed) return errno; } s->tios = tios; - fatal_signal_add_hook(restore_termios, s, true); + fatal_signal_add_hook(restore_termios, NULL, s, true); tios.c_iflag &= ~(IGNBRK | BRKINT | PARMRK | ISTRIP | INLCR | IGNCR | ICRNL | IXON); diff --git a/lib/fatal-signal.c b/lib/fatal-signal.c index 64b045d3..60a188e6 100644 --- a/lib/fatal-signal.c +++ b/lib/fatal-signal.c @@ -1,5 +1,5 @@ /* - * Copyright (c) 2008, 2009 Nicira Networks. + * Copyright (c) 2008, 2009, 2010 Nicira Networks. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -40,7 +40,8 @@ static sigset_t fatal_signal_set; /* Hooks to call upon catching a signal */ struct hook { - void (*func)(void *aux); + void (*hook_cb)(void *aux); + void (*cancel_cb)(void *aux); void *aux; bool run_at_exit; }; @@ -51,9 +52,6 @@ static size_t n_hooks; 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 fatal_signal_init(void); static void atexit_handler(void); static void call_hooks(int sig_nr); @@ -92,19 +90,29 @@ fatal_signal_init(void) } } -/* 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. +/* Registers 'hook_cb' to be called when a process termination signal is + * raised. If 'run_at_exit' is true, 'hook_cb' 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 + * 'hook_cb' 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. */ + * on signal handler functions. + * + * If the current process forks, fatal_signal_fork() may be called to clear the + * parent process's fatal signal hooks, so that 'hook_cb' is only called when + * the child terminates, not when the parent does. When fatal_signal_fork() is + * called, it calls the 'cancel_cb' function if it is nonnull, passing 'aux', + * to notify that the hook has been canceled. This allows the hook to free + * memory, etc. */ void -fatal_signal_add_hook(void (*func)(void *aux), void *aux, bool run_at_exit) +fatal_signal_add_hook(void (*hook_cb)(void *aux), void (*cancel_cb)(void *aux), + void *aux, bool run_at_exit) { fatal_signal_init(); + assert(n_hooks < MAX_HOOKS); - hooks[n_hooks].func = func; + hooks[n_hooks].hook_cb = hook_cb; + hooks[n_hooks].cancel_cb = cancel_cb; hooks[n_hooks].aux = aux; hooks[n_hooks].run_at_exit = run_at_exit; n_hooks++; @@ -150,9 +158,7 @@ fatal_signal_wait(void) static void atexit_handler(void) { - if (!disabled) { - call_hooks(0); - } + call_hooks(0); } static void @@ -167,15 +173,21 @@ call_hooks(int sig_nr) for (i = 0; i < n_hooks; i++) { struct hook *h = &hooks[i]; if (sig_nr || h->run_at_exit) { - h->func(h->aux); + h->hook_cb(h->aux); } } } } +/* Files to delete on exit. (The 'data' member of each node is unused.) */ static struct shash files = SHASH_INITIALIZER(&files); +/* Has a hook function been registered with fatal_signal_add_hook() (and not + * cleared by fatal_signal_fork())? */ +static bool added_hook; + static void unlink_files(void *aux); +static void cancel_files(void *aux); static void do_unlink_files(void); /* Registers 'file' to be unlinked when the program terminates via exit() or a @@ -183,10 +195,9 @@ static void do_unlink_files(void); void fatal_signal_add_file_to_unlink(const char *file) { - static bool added_hook = false; if (!added_hook) { added_hook = true; - fatal_signal_add_hook(unlink_files, NULL, true); + fatal_signal_add_hook(unlink_files, cancel_files, NULL, true); } if (!shash_find(&files, file)) { @@ -228,6 +239,13 @@ unlink_files(void *aux UNUSED) do_unlink_files(); } +static void +cancel_files(void *aux UNUSED) +{ + shash_clear(&files); + added_hook = false; +} + static void do_unlink_files(void) { @@ -238,23 +256,26 @@ do_unlink_files(void) } } -/* Disables the fatal signal hook mechanism. Following a fork, one of the - * resulting processes can call this function to allow it to terminate without - * triggering fatal signal processing or removing files. Fatal signal - * processing is still enabled in the other process. */ +/* Clears all of the fatal signal hooks without executing them. If any of the + * hooks passed a 'cancel_cb' function to fatal_signal_add_hook(), then those + * functions will be called, allowing them to free resources, etc. + * + * Following a fork, one of the resulting processes can call this function to + * allow it to terminate without calling the hooks registered before calling + * this function. New hooks registered after calling this function will take + * effect normally. */ void fatal_signal_fork(void) { size_t i; - disabled = true; - - for (i = 0; i < ARRAY_SIZE(fatal_signals); i++) { - int sig_nr = fatal_signals[i]; - if (signal(sig_nr, SIG_DFL) == SIG_IGN) { - signal(sig_nr, SIG_IGN); + for (i = 0; i < n_hooks; i++) { + struct hook *h = &hooks[i]; + if (h->cancel_cb) { + h->cancel_cb(h->aux); } } + n_hooks = 0; /* Raise any signals that we have already received with the default * handler. */ diff --git a/lib/fatal-signal.h b/lib/fatal-signal.h index a84d5fd7..94a1f1fb 100644 --- a/lib/fatal-signal.h +++ b/lib/fatal-signal.h @@ -1,5 +1,5 @@ /* - * Copyright (c) 2008, 2009 Nicira Networks. + * Copyright (c) 2008, 2009, 2010 Nicira Networks. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -20,7 +20,9 @@ #include /* Basic interface. */ -void fatal_signal_add_hook(void (*)(void *aux), void *aux, bool run_at_exit); +void fatal_signal_add_hook(void (*hook_cb)(void *aux), + void (*cancel_cb)(void *aux), void *aux, + bool run_at_exit); void fatal_signal_fork(void); void fatal_signal_run(void); void fatal_signal_wait(void); diff --git a/lib/netdev.c b/lib/netdev.c index 428ad950..8d243e9e 100644 --- a/lib/netdev.c +++ b/lib/netdev.c @@ -70,10 +70,11 @@ int netdev_initialize(void) { static int status = -1; + if (status < 0) { int i, j; - fatal_signal_add_hook(close_all_netdevs, NULL, true); + fatal_signal_add_hook(close_all_netdevs, NULL, NULL, true); status = 0; for (i = j = 0; i < n_netdev_classes; i++) { diff --git a/tests/test-dhcp-client.c b/tests/test-dhcp-client.c index 44730959..3b35dac6 100644 --- a/tests/test-dhcp-client.c +++ b/tests/test-dhcp-client.c @@ -1,5 +1,5 @@ /* - * Copyright (c) 2008, 2009 Nicira Networks. + * Copyright (c) 2008, 2009, 2010 Nicira Networks. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -65,7 +65,7 @@ main(int argc, char *argv[]) ovs_fatal(error, "dhclient_create failed"); } dhclient_init(cli, request_ip.s_addr); - fatal_signal_add_hook(release, cli, true); + fatal_signal_add_hook(release, NULL, cli, true); for (;;) { dhclient_run(cli); diff --git a/utilities/ovs-discover.c b/utilities/ovs-discover.c index 1c3afd06..8f46b992 100644 --- a/utilities/ovs-discover.c +++ b/utilities/ovs-discover.c @@ -102,7 +102,7 @@ main(int argc, char *argv[]) struct iface *iface = &ifaces[i]; dhclient_init(iface->dhcp, 0); } - fatal_signal_add_hook(release_ifaces, NULL, true); + fatal_signal_add_hook(release_ifaces, NULL, NULL, true); retval = regcomp(&accept_controller_regex, accept_controller_re, REG_NOSUB | REG_EXTENDED); -- 2.30.2