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.

diff --git a/extras/ezio/ovs-switchui.c b/extras/ezio/ovs-switchui.c
index 365a708..864e34c 100644 (file)
--- 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 709b802..9b05480 100644 (file)
--- 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 64b045d..60a188e 100644 (file)
--- 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);
             }
         }
     }
 }
 \f
+/* 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)
     }
 }
 \f
-/* 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 a84d5fd..94a1f1f 100644 (file)
--- 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 <stdbool.h>
 
 /* 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 428ad95..8d243e9 100644 (file)
--- 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 4473095..3b35dac 100644 (file)
--- 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 1c3afd0..8f46b99 100644 (file)
--- 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);