opr: Add new softsig implementation
authorSimon Wilkinson <sxw@your-file-system.com>
Fri, 23 Mar 2012 21:26:14 +0000 (21:26 +0000)
committerDaria Brashear <shadow@your-file-system.com>
Tue, 26 May 2015 18:06:18 +0000 (14:06 -0400)
Signals and pthreaded applications are a poor match. OpenAFS has had
the softsig system (currently in src/util/softsig.c) in an attempt to
alleviate some of these problems. However, that implementation itself
has a number of problems. It uses signal functions that are unsafe in
pthreaded applications, and uses pthread_kill within its signal
handlers. Over the years it has been responsible for a number of
portability bugs.

The old implementation continues to receive signals in the main thread
of the application. However, the handler code is run within a seperate
signal handler thread. When the main thread receives a signal a stub
handler is invoked, which simply pthread_kill()s the signal handler
thread.

The new implementation simplifies things by only receiving signals in
the handler thread. It uses only pthread-compatible signal functions,
and invokes no code from within async signal handlers.

A complete test suite is supplied.

Change-Id: I4bac68c2f853f1e7578b54ddced3833a97dd3f82
Reviewed-on: http://gerrit.openafs.org/6947
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Reviewed-by: Chas Williams <3chas3@gmail.com>
Reviewed-by: Daria Brashear <shadow@your-file-system.com>

src/opr/Makefile.in
src/opr/softsig.c [new file with mode: 0644]
src/opr/softsig.h [new file with mode: 0644]
tests/TESTS
tests/opr/.gitignore
tests/opr/Makefile.in
tests/opr/softsig-helper.c [new file with mode: 0644]
tests/opr/softsig-t [new file with mode: 0755]

index 647eaca..76ae679 100644 (file)
@@ -3,7 +3,7 @@ include @TOP_OBJDIR@/src/config/Makefile.config
 include @TOP_OBJDIR@/src/config/Makefile.pthread
 include @TOP_OBJDIR@/src/config/Makefile.libtool
 
-LT_objs = assert.lo casestrcpy.lo dict.lo fmt.lo proc.lo rbtree.lo uuid.lo
+LT_objs = assert.lo casestrcpy.lo dict.lo fmt.lo proc.lo rbtree.lo softsig.lo uuid.lo
 LT_libs = $(LIB_hcrypto) $(LIB_roken)
 
 HEADERS = $(TOP_INCDIR)/afs/opr.h \
@@ -17,6 +17,7 @@ HEADERS = $(TOP_INCDIR)/afs/opr.h \
          $(TOP_INCDIR)/opr/proc.h \
          $(TOP_INCDIR)/opr/queue.h \
          $(TOP_INCDIR)/opr/rbtree.h \
+         $(TOP_INCDIR)/opr/softsig.h \
          $(TOP_INCDIR)/opr/time.h \
          $(TOP_INCDIR)/opr/uuid.h
 
@@ -76,6 +77,9 @@ $(TOP_INCDIR)/opr/time.h: ${srcdir}/opr_time.h
 $(TOP_INCDIR)/opr/uuid.h: ${srcdir}/uuid.h
        $(INSTALL_DATA) $? $@
 
+$(TOP_INCDIR)/opr/softsig.h: softsig.h
+       $(INSTALL_DATA) $? $@
+
 clean:
        $(LT_CLEAN)
        rm -f libopr.a *.o
diff --git a/src/opr/softsig.c b/src/opr/softsig.c
new file mode 100644 (file)
index 0000000..4a210ad
--- /dev/null
@@ -0,0 +1,161 @@
+/*
+ * Copyright (c) 2010 Your File System Inc. All rights reserved.
+ *
+ * Redistribution and use in source and binary forms, with or without
+ * modification, are permitted provided that the following conditions
+ * are met:
+ * 1. Redistributions of source code must retain the above copyright
+ *    notice, this list of conditions and the following disclaimer.
+ * 2. Redistributions in binary form must reproduce the above copyright
+ *    notice, this list of conditions and the following disclaimer in the
+ *    documentation and/or other materials provided with the distribution.
+ *
+ * THIS SOFTWARE IS PROVIDED BY THE AUTHOR `AS IS'' AND ANY EXPRESS OR
+ * IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE IMPLIED WARRANTIES
+ * OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE ARE DISCLAIMED.
+ * IN NO EVENT SHALL THE AUTHOR BE LIABLE FOR ANY DIRECT, INDIRECT,
+ * INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT
+ * NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
+ * DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
+ * THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
+ * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF
+ * THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+ */
+
+/* Soft signals
+ *
+ * This is a pthread compatible signal handling system. It doesn't have any
+ * restrictions on the code which can be called from a signal handler, as handlers
+ * are processed in a dedicated thread, rather than as part of the async signal
+ * handling code.
+ *
+ * Applications wishing to use this system must call opr_softsig_Init _before_
+ * starting any other threads. After this, all signal handlers must be registered
+ * using opr_softsig_Register.
+ */
+
+#include <afsconfig.h>
+#include <afs/param.h>
+
+#include <opr.h>
+#include <roken.h>
+
+#include <pthread.h>
+
+static struct {
+    void (*handler) (int);
+} handlers[NSIG];
+
+static void
+softsigSignalSet(sigset_t *set)
+{
+    sigfillset(set);
+    sigdelset(set, SIGKILL);
+    sigdelset(set, SIGSTOP);
+    sigdelset(set, SIGCONT);
+    sigdelset(set, SIGABRT);
+    sigdelset(set, SIGBUS);
+    sigdelset(set, SIGFPE);
+    sigdelset(set, SIGILL);
+    sigdelset(set, SIGPIPE);
+    sigdelset(set, SIGSEGV);
+    sigdelset(set, SIGTRAP);
+}
+
+static void *
+signalHandler(void *arg)
+{
+    int receivedSignal;
+    sigset_t set;
+
+    softsigSignalSet(&set);
+    while (1) {
+       opr_Verify(sigwait(&set, &receivedSignal) == 0);
+       opr_Verify(sigismember(&set, receivedSignal) == 1);
+       if (handlers[receivedSignal].handler != NULL) {
+           handlers[receivedSignal].handler(receivedSignal);
+       }
+    }
+    return NULL;
+}
+
+static void
+ExitHandler(int signal)
+{
+    exit(signal);
+}
+
+static void
+StopHandler(int signal)
+{
+    kill(getpid(), SIGSTOP);
+}
+
+/*!
+ * Register a soft signal handler
+ *
+ * Soft signal handlers may only be registered for async signals.
+ *
+ * @param[in] sig
+ *      The signal to register a handler for.
+ * @param[in] handler
+ *      The handler function to register, or NULL, to clear a signal handler.
+ *
+ * @returns
+ *      EINVAL if the signal given isn't one for which we can register a soft
+ *      handler.
+ */
+
+int
+opr_softsig_Register(int sig, void (*handler)(int))
+{
+    sigset_t set;
+
+    softsigSignalSet(&set);
+
+    /* Check that the supplied signal is handled by softsig. */
+    if (sigismember(&set, sig)) {
+       handlers[sig].handler = handler;
+       return 0;
+    }
+
+    return EINVAL;
+}
+
+/*!
+ * Initialise the soft signal system
+ *
+ * This call initialises the soft signal system. It provides default handlers for
+ * SIGINT and SIGTSTP which preserve the operating system behaviour (terminating
+ * and stopping the process, respectively).
+ *
+ * opr_softsig_Init() must be called before any threads are created, as it sets
+ * up a global signal mask on the parent process that is then inherited by all
+ * children.
+ */
+
+int
+opr_softsig_Init(void)
+{
+    sigset_t set;
+    pthread_t handlerThread;
+
+    /* Block all signals in the main thread, and in any threads which are created
+     * after us. Only the signal handler thread will receive signals. */
+    softsigSignalSet(&set);
+    pthread_sigmask(SIG_BLOCK, &set, NULL);
+
+    /* Register a few handlers so that we keep the usual behaviour for CTRL-C and
+     * CTRL-Z, unless the application replaces them. */
+    opr_Verify(opr_softsig_Register(SIGINT, ExitHandler) == 0);
+    opr_Verify(opr_softsig_Register(SIGTERM, ExitHandler) == 0);
+    opr_Verify(opr_softsig_Register(SIGQUIT, ExitHandler) == 0);
+    opr_Verify(opr_softsig_Register(SIGTSTP, StopHandler) == 0);
+
+    /* Create a signal handler thread which will respond to any incoming signals
+     * for us. */
+    opr_Verify(pthread_create(&handlerThread, NULL, signalHandler, NULL) == 0);
+    opr_Verify(pthread_detach(handlerThread) == 0);
+
+    return 0;
+}
diff --git a/src/opr/softsig.h b/src/opr/softsig.h
new file mode 100644 (file)
index 0000000..dd9d09b
--- /dev/null
@@ -0,0 +1,31 @@
+/*
+ * Copyright (c) 2012 Your File System Inc. All rights reserved.
+ *
+ * Redistribution and use in source and binary forms, with or without
+ * modification, are permitted provided that the following conditions
+ * are met:
+ * 1. Redistributions of source code must retain the above copyright
+ *    notice, this list of conditions and the following disclaimer.
+ * 2. Redistributions in binary form must reproduce the above copyright
+ *    notice, this list of conditions and the following disclaimer in the
+ *    documentation and/or other materials provided with the distribution.
+ *
+ * THIS SOFTWARE IS PROVIDED BY THE AUTHOR `AS IS'' AND ANY EXPRESS OR
+ * IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE IMPLIED WARRANTIES
+ * OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE ARE DISCLAIMED.
+ * IN NO EVENT SHALL THE AUTHOR BE LIABLE FOR ANY DIRECT, INDIRECT,
+ * INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT
+ * NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
+ * DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
+ * THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
+ * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF
+ * THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+ */
+
+#ifndef OPENAFS_OPR_SOFTSIG_H
+#define OPENAFS_OPR_SOFTSIG_H 1
+
+int opr_softsig_Init(void);
+int opr_softsig_Register(int sig, void (*handler)(int));
+
+#endif
index f8f4e1f..e59014e 100644 (file)
@@ -10,6 +10,7 @@ opr/fmt
 opr/jhash
 opr/queues
 opr/rbtree
+opr/softsig
 opr/time
 opr/uuid
 ptserver/pt_util
index 1d8be83..1941a93 100644 (file)
@@ -9,3 +9,4 @@
 /rbtree-t
 /time-t
 /uuid-t
+/softsig-helper
index 7625eb4..9660177 100644 (file)
@@ -7,7 +7,7 @@ MODULE_CFLAGS = -I$(srcdir)/../..
 
 LIBS=../tap/libtap.a $(abs_top_builddir)/src/opr/liboafs_opr.la
 
-tests = dict-t fmt-t jhash-t queues-t rbtree-t time-t uuid-t
+tests = dict-t fmt-t jhash-t queues-t rbtree-t softsig-helper time-t uuid-t
 
 all check test tests: $(tests)
 
@@ -32,6 +32,9 @@ time-t: time-t.o
 uuid-t: uuid-t.o
        $(LT_LDRULE_static) uuid-t.o ../tap/libtap.a $(LIBS) $(XLIBS)
 
+softsig-helper: softsig-helper.o $(LIBS)
+       $(LT_LDRULE_static) softsig-helper.o $(LIBS) $(XLIBS)
+
 clean distclean:
        $(LT_CLEAN)
        $(RM) -f $(tests) *.o core
diff --git a/tests/opr/softsig-helper.c b/tests/opr/softsig-helper.c
new file mode 100644 (file)
index 0000000..cffb997
--- /dev/null
@@ -0,0 +1,140 @@
+/*
+ * Copyright (c) 2012 Your File System Inc. All rights reserved.
+ *
+ * Redistribution and use in source and binary forms, with or without
+ * modification, are permitted provided that the following conditions
+ * are met:
+ * 1. Redistributions of source code must retain the above copyright
+ *    notice, this list of conditions and the following disclaimer.
+ * 2. Redistributions in binary form must reproduce the above copyright
+ *    notice, this list of conditions and the following disclaimer in the
+ *    documentation and/or other materials provided with the distribution.
+ *
+ * THIS SOFTWARE IS PROVIDED BY THE AUTHOR `AS IS'' AND ANY EXPRESS OR
+ * IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE IMPLIED WARRANTIES
+ * OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE ARE DISCLAIMED.
+ * IN NO EVENT SHALL THE AUTHOR BE LIABLE FOR ANY DIRECT, INDIRECT,
+ * INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT
+ * NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
+ * DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
+ * THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
+ * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF
+ * THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+ */
+
+/* A test helper for the softsig code. This just sits and reports what signals
+ * it has received.
+ */
+
+#include <afsconfig.h>
+#include <afs/param.h>
+
+#include <roken.h>
+#include <pthread.h>
+#include <sys/mman.h>
+
+#include <afs/opr.h>
+#include <opr/softsig.h>
+
+#define SIGTABLEENTRY(name)    { #name, SIG##name }
+
+static struct sigtable {
+       char *name;
+       int signo;
+} sigtable[] = {
+       SIGTABLEENTRY(INT),
+       SIGTABLEENTRY(HUP),
+       SIGTABLEENTRY(QUIT),
+       SIGTABLEENTRY(ALRM),
+       SIGTABLEENTRY(TERM),
+       SIGTABLEENTRY(TSTP),
+       SIGTABLEENTRY(USR1),
+       SIGTABLEENTRY(USR2)
+       };
+
+static char *signame(int signo) {
+    int i;
+
+    for (i = 0; i < sizeof(sizeof(sigtable) / sizeof(sigtable[0])); ++i) {
+       if (sigtable[i].signo == signo) {
+           return sigtable[i].name;
+       }
+    }
+
+    return "UNK";
+}
+
+static void
+handler(int signal) {
+    printf("Received %s\n", signame(signal));
+
+    fflush(stdout);
+}
+
+static void *
+mythread(void *arg) {
+    while (1) {
+       sleep(60);
+    }
+    return NULL;
+}
+
+static void *
+crashingThread(void *arg) {
+    *(char *)1 = 'a';  /* raises SIGSEGV */
+    return NULL; /* Ha! */
+}
+
+static void *
+thrownUnderTheBusThread(void *arg) {
+    char *path = arg;
+    int fd = open(path, O_CREAT | O_APPEND, 0660);
+    char *m = mmap(NULL, 10, PROT_WRITE, MAP_PRIVATE, fd, 0);
+    *(m + 11) = 'a';  /* raises SIGBUS */
+    return NULL;
+}
+
+
+int
+main(int argvc, char **argv)
+{
+    char *path;
+    int i;
+    int threads = 10;
+
+    opr_softsig_Init();
+
+    opr_Verity(opr_softsig_Register(SIGINT, handler) == 0);
+    opr_Verity(opr_softsig_Register(SIGHUP, handler) == 0);
+    opr_Verity(opr_softsig_Register(SIGQUIT, handler) == 0);
+    opr_Verity(opr_softsig_Register(SIGALRM, handler) == 0);
+    opr_Verity(opr_softsig_Register(SIGTERM, handler) == 0);
+    opr_Verity(opr_softsig_Register(SIGTSTP, handler) == 0);
+    opr_Verity(opr_softsig_Register(SIGUSR1, handler) == 0);
+    opr_Verity(opr_softsig_Register(SIGUSR2, handler) == 0);
+
+    for (i=0; i<threads; i++) {
+       pthread_t id;
+       opr_Verify(pthread_create(&id, NULL, mythread, NULL) == 0);
+    }
+
+    if (argvc>1 && strcmp(argv[1], "-crash") == 0) {
+       pthread_t id;
+       opr_Verify(pthread_create(&id, NULL, crashingThread, NULL) == 0);
+    } else if (argvc>1 && strcmp(argv[1], "-buserror") == 0) {
+       if (argvc > 2)
+           path = argv[2];
+       else
+           opr_abort();
+       pthread_t id;
+       opr_Verify(pthread_create(&id, NULL, thrownUnderTheBusThread,
+                                 path) == 0);
+    } else {
+       printf("Ready\n");
+       fflush(stdout);
+    }
+
+    mythread(NULL);
+
+    return 0;
+}
diff --git a/tests/opr/softsig-t b/tests/opr/softsig-t
new file mode 100755 (executable)
index 0000000..58c7a61
--- /dev/null
@@ -0,0 +1,86 @@
+#!/usr/bin/perl
+#
+# Copyright (c) 2010 Your File System Inc. All rights reserved.
+#
+# Redistribution and use in source and binary forms, with or without
+# modification, are permitted provided that the following conditions
+# are met:
+# 1. Redistributions of source code must retain the above copyright
+#    notice, this list of conditions and the following disclaimer.
+# 2. Redistributions in binary form must reproduce the above copyright
+#    notice, this list of conditions and the following disclaimer in the
+#    documentation and/or other materials provided with the distribution.
+#
+# THIS SOFTWARE IS PROVIDED BY THE AUTHOR `AS IS'' AND ANY EXPRESS OR
+# IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE IMPLIED WARRANTIES
+# OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE ARE DISCLAIMED.
+# IN NO EVENT SHALL THE AUTHOR BE LIABLE FOR ANY DIRECT, INDIRECT,
+# INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT
+# NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
+# DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
+# THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
+# (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF
+# THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+
+use strict;
+use warnings;
+use Test::More tests => 11;
+use IO::File;
+use POSIX qw(:signal_h);
+use File::Temp;
+
+# Start up our test process, and send it various signals. Check that these
+# signals make it to it correctly, and are reported on the command line.
+
+my $pid=open(HELPER, "./softsig-helper |")
+    or die "Couldn't start test helper.";
+
+# Wait for softsig to start up.
+is(<HELPER>, "Ready\n");
+
+# Check that a load of common signals are correctly trapped.
+
+kill 'INT', $pid;
+is(<HELPER>, "Received INT\n");
+
+kill 'HUP', $pid;
+is(<HELPER>, "Received HUP\n");
+
+kill 'QUIT', $pid;
+is(<HELPER>, "Received QUIT\n");
+
+kill 'ALRM', $pid;
+is(<HELPER>, "Received ALRM\n");
+
+kill 'TERM', $pid;
+is(<HELPER>, "Received TERM\n");
+
+kill 'USR1', $pid;
+is(<HELPER>, "Received USR1\n");
+
+kill 'USR2', $pid;
+is(<HELPER>, "Received USR2\n");
+
+
+# Check that we can actually stop the process with a kill.
+
+kill 'KILL', $pid;
+close(HELPER);
+is($?, SIGKILL, "Helper exited on KILL signal.");
+
+# Check that an internal segmentation fault kills the process.
+
+$pid = open(HELPER, "./softsig-helper -crash |")
+    or die "Couldn't start test helper.";
+close(HELPER);
+is($? & 0x7f, SIGSEGV, "Helper exited on SEGV signal.");
+
+# Check that an internal bus error kills the process.
+
+my ($fh, $path) = mkstemp("/tmp/softsig-t_XXXXXX");
+$pid = open(HELPER, "./softsig-helper -buserror $path |")
+    or die "Couldn't start test helper.";
+close(HELPER);
+is($? & 0x7f, SIGBUS, "Helper exited on BUS signal.");
+$fh->close;
+unlink $path;