From: Simon Wilkinson Date: Fri, 23 Mar 2012 21:26:14 +0000 (+0000) Subject: opr: Add new softsig implementation X-Git-Tag: openafs-stable-1_8_0pre1~320 X-Git-Url: https://git.openafs.org/?p=openafs.git;a=commitdiff_plain;h=cb0081604ef5369f34279c6eb77eb4d28406f2ac opr: Add new softsig implementation 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 Reviewed-by: Chas Williams <3chas3@gmail.com> Reviewed-by: Daria Brashear --- diff --git a/src/opr/Makefile.in b/src/opr/Makefile.in index 647eaca..76ae679 100644 --- a/src/opr/Makefile.in +++ b/src/opr/Makefile.in @@ -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 index 0000000..4a210ad --- /dev/null +++ b/src/opr/softsig.c @@ -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 +#include + +#include +#include + +#include + +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 index 0000000..dd9d09b --- /dev/null +++ b/src/opr/softsig.h @@ -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 diff --git a/tests/TESTS b/tests/TESTS index f8f4e1f..e59014e 100644 --- a/tests/TESTS +++ b/tests/TESTS @@ -10,6 +10,7 @@ opr/fmt opr/jhash opr/queues opr/rbtree +opr/softsig opr/time opr/uuid ptserver/pt_util diff --git a/tests/opr/.gitignore b/tests/opr/.gitignore index 1d8be83..1941a93 100644 --- a/tests/opr/.gitignore +++ b/tests/opr/.gitignore @@ -9,3 +9,4 @@ /rbtree-t /time-t /uuid-t +/softsig-helper diff --git a/tests/opr/Makefile.in b/tests/opr/Makefile.in index 7625eb4..9660177 100644 --- a/tests/opr/Makefile.in +++ b/tests/opr/Makefile.in @@ -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 index 0000000..cffb997 --- /dev/null +++ b/tests/opr/softsig-helper.c @@ -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 +#include + +#include +#include +#include + +#include +#include + +#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; i1 && 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 index 0000000..58c7a61 --- /dev/null +++ b/tests/opr/softsig-t @@ -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(, "Ready\n"); + +# Check that a load of common signals are correctly trapped. + +kill 'INT', $pid; +is(, "Received INT\n"); + +kill 'HUP', $pid; +is(, "Received HUP\n"); + +kill 'QUIT', $pid; +is(, "Received QUIT\n"); + +kill 'ALRM', $pid; +is(, "Received ALRM\n"); + +kill 'TERM', $pid; +is(, "Received TERM\n"); + +kill 'USR1', $pid; +is(, "Received USR1\n"); + +kill 'USR2', $pid; +is(, "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;