Looks good to me, thanks,
On Thu, Jul 18, 2013 at 4:15 PM, Ben Pfaff <b...@nicira.com> wrote:
> Signed-off-by: Ben Pfaff <b...@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
>
> _______________________________________________
> dev mailing list
> dev@openvswitch.org
> http://openvswitch.org/mailman/listinfo/dev
>
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
