[ovs-dev] [threads 22/23] fatal-signal: Make thread-safe.

Ben Pfaff blp at nicira.com
Thu Jul 18 23:15:35 UTC 2013


Signed-off-by: Ben Pfaff <blp at nicira.com>
---
 lib/fatal-signal.c |   56 +++++++++++++++++++++++++++++++++++++++++++--------
 lib/fatal-signal.h |    3 +-
 lib/ovs-thread.c   |    5 ++++
 lib/ovs-thread.h   |    6 +++++
 4 files changed, 60 insertions(+), 10 deletions(-)

diff --git a/lib/fatal-signal.c b/lib/fatal-signal.c
index db8d98e..40daf5a 100644
--- a/lib/fatal-signal.c
+++ b/lib/fatal-signal.c
@@ -23,6 +23,7 @@
 #include <stdlib.h>
 #include <string.h>
 #include <unistd.h>
+#include "ovs-thread.h"
 #include "poll-loop.h"
 #include "shash.h"
 #include "sset.h"
@@ -56,20 +57,32 @@ static size_t n_hooks;
 static int signal_fds[2];
 static volatile sig_atomic_t stored_sig_nr = SIG_ATOMIC_MAX;
 
-static void fatal_signal_init(void);
+static pthread_mutex_t mutex;
+
 static void atexit_handler(void);
 static void call_hooks(int sig_nr);
 
-static void
+/* Initializes the fatal signal handling module.  Calling this function is
+ * optional, because calling any other function in the module will also
+ * initialize it.  However, in a multithreaded program, the module must be
+ * initialized while the process is still single-threaded. */
+void
 fatal_signal_init(void)
 {
     static bool inited = false;
 
     if (!inited) {
+        pthread_mutexattr_t attr;
         size_t i;
 
+        assert_single_threaded();
         inited = true;
 
+        xpthread_mutexattr_init(&attr);
+        xpthread_mutexattr_settype(&attr, PTHREAD_MUTEX_RECURSIVE);
+        xpthread_mutex_init(&mutex, &attr);
+        xpthread_mutexattr_destroy(&attr);
+
         xpipe_nonblocking(signal_fds);
 
         for (i = 0; i < ARRAY_SIZE(fatal_signals); i++) {
@@ -86,13 +99,13 @@ fatal_signal_init(void)
     }
 }
 
-/* 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.
+/* Registers 'hook_cb' to be called from inside poll_block() following a fatal
+ * signal.  'hook_cb' does not need to be async-signal-safe.  In a
+ * multithreaded program 'hook_cb' might be called from any thread, with
+ * threads other than the one running 'hook_cb' in unknown states.
  *
- * '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.
+ * 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.
  *
  * 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
@@ -106,12 +119,14 @@ fatal_signal_add_hook(void (*hook_cb)(void *aux), void (*cancel_cb)(void *aux),
 {
     fatal_signal_init();
 
+    xpthread_mutex_lock(&mutex);
     ovs_assert(n_hooks < MAX_HOOKS);
     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++;
+    xpthread_mutex_unlock(&mutex);
 }
 
 /* Handles fatal signal number 'sig_nr'.
@@ -152,6 +167,8 @@ fatal_signal_run(void)
     if (sig_nr != SIG_ATOMIC_MAX) {
         char namebuf[SIGNAL_NAME_BUFSIZE];
 
+        xpthread_mutex_lock(&mutex);
+
         VLOG_WARN("terminating with signal %d (%s)",
                   (int)sig_nr, signal_name(sig_nr, namebuf, sizeof namebuf));
         call_hooks(sig_nr);
@@ -160,6 +177,9 @@ fatal_signal_run(void)
          * termination status reflects that we were killed by this signal */
         signal(sig_nr, SIG_DFL);
         raise(sig_nr);
+
+        xpthread_mutex_unlock(&mutex);
+        NOT_REACHED();
     }
 }
 
@@ -210,12 +230,16 @@ static void do_unlink_files(void);
 void
 fatal_signal_add_file_to_unlink(const char *file)
 {
+    fatal_signal_init();
+
+    xpthread_mutex_lock(&mutex);
     if (!added_hook) {
         added_hook = true;
         fatal_signal_add_hook(unlink_files, cancel_files, NULL, true);
     }
 
     sset_add(&files, file);
+    xpthread_mutex_unlock(&mutex);
 }
 
 /* Unregisters 'file' from being unlinked when the program terminates via
@@ -223,7 +247,11 @@ fatal_signal_add_file_to_unlink(const char *file)
 void
 fatal_signal_remove_file_to_unlink(const char *file)
 {
+    fatal_signal_init();
+
+    xpthread_mutex_lock(&mutex);
     sset_find_and_delete(&files, file);
+    xpthread_mutex_unlock(&mutex);
 }
 
 /* Like fatal_signal_remove_file_to_unlink(), but also unlinks 'file'.
@@ -231,13 +259,21 @@ fatal_signal_remove_file_to_unlink(const char *file)
 int
 fatal_signal_unlink_file_now(const char *file)
 {
-    int error = unlink(file) ? errno : 0;
+    int error;
+
+    fatal_signal_init();
+
+    xpthread_mutex_lock(&mutex);
+
+    error = unlink(file) ? errno : 0;
     if (error) {
         VLOG_WARN("could not unlink \"%s\" (%s)", file, ovs_strerror(error));
     }
 
     fatal_signal_remove_file_to_unlink(file);
 
+    xpthread_mutex_unlock(&mutex);
+
     return error;
 }
 
@@ -277,6 +313,8 @@ fatal_signal_fork(void)
 {
     size_t i;
 
+    assert_single_threaded();
+
     for (i = 0; i < n_hooks; i++) {
         struct hook *h = &hooks[i];
         if (h->cancel_cb) {
diff --git a/lib/fatal-signal.h b/lib/fatal-signal.h
index 8a1a84b..b458d3d 100644
--- a/lib/fatal-signal.h
+++ b/lib/fatal-signal.h
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2008, 2009, 2010 Nicira, Inc.
+ * Copyright (c) 2008, 2009, 2010, 2013 Nicira, Inc.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -20,6 +20,7 @@
 #include <stdbool.h>
 
 /* Basic interface. */
+void fatal_signal_init(void);
 void fatal_signal_add_hook(void (*hook_cb)(void *aux),
                            void (*cancel_cb)(void *aux), void *aux,
                            bool run_at_exit);
diff --git a/lib/ovs-thread.c b/lib/ovs-thread.c
index d08751c..4776587 100644
--- a/lib/ovs-thread.c
+++ b/lib/ovs-thread.c
@@ -77,6 +77,11 @@ XPTHREAD_FUNC1(pthread_mutex_lock, pthread_mutex_t *);
 XPTHREAD_FUNC1(pthread_mutex_unlock, pthread_mutex_t *);
 XPTHREAD_TRY_FUNC1(pthread_mutex_trylock, pthread_mutex_t *);
 
+XPTHREAD_FUNC1(pthread_mutexattr_init, pthread_mutexattr_t *);
+XPTHREAD_FUNC1(pthread_mutexattr_destroy, pthread_mutexattr_t *);
+XPTHREAD_FUNC2(pthread_mutexattr_settype, pthread_mutexattr_t *, int);
+XPTHREAD_FUNC2(pthread_mutexattr_gettype, pthread_mutexattr_t *, int *);
+
 XPTHREAD_FUNC2(pthread_rwlock_init,
                pthread_rwlock_t *, pthread_rwlockattr_t *);
 XPTHREAD_FUNC1(pthread_rwlock_rdlock, pthread_rwlock_t *);
diff --git a/lib/ovs-thread.h b/lib/ovs-thread.h
index 9c8023e..b18447d 100644
--- a/lib/ovs-thread.h
+++ b/lib/ovs-thread.h
@@ -56,11 +56,17 @@
  * process with an error message on any error.  The *_trylock() functions are
  * exceptions: they pass through a 0 or EBUSY return value to the caller and
  * abort on any other error. */
+
 void xpthread_mutex_init(pthread_mutex_t *, pthread_mutexattr_t *);
 void xpthread_mutex_lock(pthread_mutex_t *mutex) OVS_ACQUIRES(mutex);
 void xpthread_mutex_unlock(pthread_mutex_t *mutex) OVS_RELEASES(mutex);
 int xpthread_mutex_trylock(pthread_mutex_t *);
 
+void xpthread_mutexattr_init(pthread_mutexattr_t *);
+void xpthread_mutexattr_destroy(pthread_mutexattr_t *);
+void xpthread_mutexattr_settype(pthread_mutexattr_t *, int type);
+void xpthread_mutexattr_gettype(pthread_mutexattr_t *, int *typep);
+
 void xpthread_rwlock_init(pthread_rwlock_t *, pthread_rwlockattr_t *);
 void xpthread_rwlock_rdlock(pthread_rwlock_t *rwlock) OVS_ACQUIRES(rwlock);
 void xpthread_rwlock_wrlock(pthread_rwlock_t *rwlock) OVS_ACQUIRES(rwlock);
-- 
1.7.2.5




More information about the dev mailing list