Improve robustness of sigprocmask by overriding signal.

* lib/signal.in.h (rpl_signal): Override signal when sigprocmask
is in use.
* lib/sigprocmask.c (blocked_handler): Reinstall block handler.
(SIGKILL, SIGSTOP): Provide fallbacks.
(rpl_signal): Implement.
(old_handlers, blocked_set): Mark volatile, since sigprocmask and
signal can be called inside handlers.

Signed-off-by: Eric Blake <ebb9@byu.net>

diff --git a/ChangeLog b/ChangeLog
index 9f4d999..7d08ae6 100644 (file)
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,5 +1,14 @@
 2008-06-21  Eric Blake  <ebb9@byu.net>
 
+       Improve robustness of sigprocmask by overriding signal.
+       * lib/signal.in.h (rpl_signal): Override signal when sigprocmask
+       is in use.
+       * lib/sigprocmask.c (blocked_handler): Reinstall block handler.
+       (SIGKILL, SIGSTOP): Provide fallbacks.
+       (rpl_signal): Implement.
+       (old_handlers, blocked_set): Mark volatile, since sigprocmask and
+       signal can be called inside handlers.
+
        Fix nanosleep module on mingw.
        * modules/nanosleep (Depends-on): Add sys_select.
        * lib/nanosleep.c (HAVE_SYS_SELECT_H): Rely on gnulib module.

diff --git a/lib/signal.in.h b/lib/signal.in.h
index 9f82ba7..eb16afb 100644 (file)
--- a/lib/signal.in.h
+++ b/lib/signal.in.h
@@ -1,6 +1,6 @@
 /* A GNU-like <signal.h>.
 
-   Copyright (C) 2006-2007 Free Software Foundation, Inc.
+   Copyright (C) 2006-2008 Free Software Foundation, Inc.
 
    This program is free software: you can redistribute it and/or modify
    it under the terms of the GNU General Public License as published by
@@ -33,6 +33,10 @@
 
 /* The definition of GL_LINK_WARNING is copied here.  */
 
+/* Mingw defines sigset_t not in <signal.h>, but in <sys/types.h>.  */
+#if !@HAVE_POSIX_SIGNALBLOCKING@
+# include <sys/types.h>
+#endif
 
 #ifdef __cplusplus
 extern "C" {
@@ -41,9 +45,6 @@ extern "C" {
 
 #if !@HAVE_POSIX_SIGNALBLOCKING@
 
-/* Mingw defines sigset_t not in <signal.h>, but in <sys/types.h>.  */
-# include <sys/types.h>
-
 /* Maximum signal number + 1.  */
 # ifndef NSIG
 #  define NSIG 32
@@ -85,6 +86,11 @@ extern int sigpending (sigset_t *set);
 # define SIG_UNBLOCK 2  /* blocked_set = blocked_set & ~*set; */
 extern int sigprocmask (int operation, const sigset_t *set, sigset_t *old_set);
 
+# define signal rpl_signal
+/* Install the handler FUNC for signal SIG, and return the previous
+   handler.  */
+extern void (*signal (int sig, void (*func) (int))) (int);
+
 #endif
 
 

diff --git a/lib/sigprocmask.c b/lib/sigprocmask.c
index 456545a..87d585c 100644 (file)
--- a/lib/sigprocmask.c
+++ b/lib/sigprocmask.c
@@ -24,9 +24,24 @@
 #include <stdint.h>
 #include <stdlib.h>
 
-/* We assume that a platform without POSIX signal blocking functions also
-   does not have the POSIX sigaction() function, only the signal() function.
-   This is true for Woe32 platforms.  */
+/* We assume that a platform without POSIX signal blocking functions
+   also does not have the POSIX sigaction() function, only the
+   signal() function.  We also assume signal() has SysV semantics,
+   where any handler is uninstalled prior to being invoked.  This is
+   true for Woe32 platforms.  */
+
+/* We use raw signal(), but also provide a wrapper rpl_signal() so
+   that applications can query or change a blocked signal.  */
+#undef signal
+
+/* Provide invalid signal numbers as fallbacks if the uncatchable
+   signals are not defined.  */
+#ifndef SIGKILL
+# define SIGKILL (-1)
+#endif
+#ifndef SIGSTOP
+# define SIGSTOP (-1)
+#endif
 
 /* A signal handler.  */
 typedef void (*handler_t) (int signal);
@@ -85,7 +100,7 @@ sigfillset (sigset_t *set)
 }
 
 /* Set of currently blocked signals.  */
-static sigset_t blocked_set /* = 0 */;
+static volatile sigset_t blocked_set /* = 0 */;
 
 /* Set of currently blocked and pending signals.  */
 static volatile sig_atomic_t pending_array[NSIG] /* = { 0 } */;
@@ -94,6 +109,12 @@ static volatile sig_atomic_t pending_array[NSIG] /* = { 0 } */;
 static void
 blocked_handler (int sig)
 {
+  /* Reinstall the handler, in case the signal occurs multiple times
+     while blocked.  There is an inherent race where an asynchronous
+     signal in between when the kernel uninstalled the handler and
+     when we reinstall it will trigger the default handler; oh
+     well.  */
+  signal (sig, blocked_handler);
   if (sig >= 0 && sig < NSIG)
     pending_array[sig] = 1;
 }
@@ -113,7 +134,7 @@ sigpending (sigset_t *set)
 
 /* The previous signal handlers.
    Only the array elements corresponding to blocked signals are relevant.  */
-static handler_t old_handlers[NSIG];
+static volatile handler_t old_handlers[NSIG];
 
 int
 sigprocmask (int operation, const sigset_t *set, sigset_t *old_set)
@@ -184,3 +205,37 @@ sigprocmask (int operation, const sigset_t *set, sigset_t *old_set)
     }
   return 0;
 }
+
+/* Install the handler FUNC for signal SIG, and return the previous
+   handler.  */
+handler_t
+rpl_signal (int sig, handler_t handler)
+{
+  /* We must provide a wrapper, so that a user can query what handler
+     they installed even if that signal is currently blocked.  */
+  if (sig >= 0 && sig < NSIG && sig != SIGKILL && sig != SIGSTOP
+      && handler != SIG_ERR)
+    {
+      if (blocked_set & (1U << sig))
+       {
+         /* POSIX states that sigprocmask and signal are both
+            async-signal-safe.  This is not true of our
+            implementation - there is a slight data race where an
+            asynchronous interrupt on signal A can occur after we
+            install blocked_handler but before we have updated
+            old_handlers for signal B, such that handler A can see
+            stale information if it calls signal(B).  Oh well -
+            signal handlers really shouldn't try to manipulate the
+            installed handlers of unrelated signals.  */
+         handler_t result = old_handlers[sig];
+         old_handlers[sig] = handler;
+         return result;
+       }
+      return signal (sig, handler);
+    }
+  else
+    {
+      errno = EINVAL;
+      return SIG_ERR;
+    }
+}