Fixed thread safety of signal handling.

Signals delivery is generally not deterministic in multi-threaded programs,
but SIGSEGV is always sent by the kernel to the thread which caused the fault.

Replaced the global signal handler array by a pointer in the per-thread
context and replaced saving of original handles (which was wrong even in
some non-threaded cases) by locked use counters.

Also moved logging of the SIGSEGV error message out of the signal handler
to be sure there are no clashes.

When libucw is compiled without threading, everything is nearly as simple
as before, because the thread context struct is static and all locks
disappear.

diff --git a/lib/THREADS b/lib/THREADS
index 69452b573b6d7f87611ad34c2cbd69eeac9fa9fa..ec58d49a10cef7fc55dd0122b4ebc4e564716cd4 100644 (file)
--- a/lib/THREADS
+++ b/lib/THREADS
@@ -5,4 +5,3 @@ which also includes functions acting on any kind of global state.
 There are some exceptions:
 
 - setproctitle() is not safe, it modifies global state
-- handle_signal() is not safe, it modifies global state

diff --git a/lib/lib.h b/lib/lib.h
index 21fe4773d517821d735a06bf666b7c1467b1b433..27a7511b1308260c67196a23dc87a48fee16bc96 100644 (file)
--- a/lib/lib.h
+++ b/lib/lib.h
@@ -259,13 +259,11 @@ void sync_dir(byte *name);
 
 /* sighandler.c */
 
-typedef int (*sh_sighandler_t)(int);
-  /* obtains signum, returns nonzero if abort() should be called */
-extern sh_sighandler_t signal_handler[];
+typedef int (*sh_sighandler_t)(int);   // gets signum, returns nonzero if abort() should be called
 
-struct sigaction;
-void handle_signal(int signum, struct sigaction *oldact);
-void unhandle_signal(int signum, struct sigaction *oldact);
+void handle_signal(int signum);
+void unhandle_signal(int signum);
+sh_sighandler_t set_signal_handler(int signum, sh_sighandler_t new);
 
 /* string.c */
 

diff --git a/lib/lizard-safe.c b/lib/lizard-safe.c
index 84e52da18e54dd5f87f84fba75dd12d4d186671e..ebefa154786d31870fa5c0f12143c6ea71bcbe23 100644 (file)
--- a/lib/lizard-safe.c
+++ b/lib/lizard-safe.c
@@ -8,6 +8,7 @@
  */
 
 #include "lib/lib.h"
+#include "lib/threads.h"
 #include "lib/lizard.h"
 
 #include <sys/mman.h>
@@ -20,7 +21,6 @@
 struct lizard_buffer {
   uns len;
   void *ptr;
-  struct sigaction old_sigsegv_handler;
 };
 
 struct lizard_buffer *
@@ -29,16 +29,16 @@ lizard_alloc(void)
   struct lizard_buffer *buf = xmalloc(sizeof(struct lizard_buffer));
   buf->len = 0;
   buf->ptr = NULL;
-  handle_signal(SIGSEGV, &buf->old_sigsegv_handler);
+  handle_signal(SIGSEGV);
   return buf;
 }
 
 void
 lizard_free(struct lizard_buffer *buf)
 {
+  unhandle_signal(SIGSEGV);
   if (buf->ptr)
     munmap(buf->ptr, buf->len + PAGE_SIZE);
-  unhandle_signal(SIGSEGV, &buf->old_sigsegv_handler);
   xfree(buf);
 }
 
@@ -56,7 +56,7 @@ lizard_realloc(struct lizard_buffer *buf, uns max_len)
   buf->len = max_len;
   buf->ptr = mmap(NULL, buf->len + PAGE_SIZE, PROT_READ | PROT_WRITE, MAP_ANON | MAP_PRIVATE, -1, 0);
   if (buf->ptr == MAP_FAILED)
-    die("mmap(anonymous): %m");
+    die("mmap(anonymous, %d bytes): %m", buf->len + PAGE_SIZE);
   if (mprotect(buf->ptr + buf->len, PAGE_SIZE, PROT_NONE) < 0)
     die("mprotect: %m");
 }
@@ -65,7 +65,6 @@ static jmp_buf safe_decompress_jump;
 static int
 sigsegv_handler(int signal UNUSED)
 {
-  log(L_ERROR, "SIGSEGV caught in lizard_decompress()");
   longjmp(safe_decompress_jump, 1);
   return 1;
 }
@@ -81,8 +80,7 @@ lizard_decompress_safe(byte *in, struct lizard_buffer *buf, uns expected_length)
   uns lock_offset = ALIGN_TO(expected_length + 3, PAGE_SIZE);  // +3 due to the unaligned access
   if (lock_offset > buf->len)
     lizard_realloc(buf, lock_offset);
-  volatile sh_sighandler_t old_handler = signal_handler[SIGSEGV];
-  signal_handler[SIGSEGV] = sigsegv_handler;
+  volatile sh_sighandler_t old_handler = set_signal_handler(SIGSEGV, sigsegv_handler);
   byte *ptr;
   if (!setjmp(safe_decompress_jump))
   {
@@ -96,9 +94,10 @@ lizard_decompress_safe(byte *in, struct lizard_buffer *buf, uns expected_length)
   }
   else
   {
+    log(L_ERROR, "SIGSEGV caught in lizard_decompress()");
     ptr = NULL;
     errno = EFAULT;
   }
-  signal_handler[SIGSEGV] = old_handler;
+  set_signal_handler(SIGSEGV, old_handler);
   return ptr;
 }

diff --git a/lib/sighandler.c b/lib/sighandler.c
index 4d991d8562f4b7cdfe70a44d7f8978b1a5037d4f..4991f9a3379b080fb0681cac303c24abdf138125 100644 (file)
--- a/lib/sighandler.c
+++ b/lib/sighandler.c
@@ -2,41 +2,63 @@
  *     UCW Library -- Catching of signals and calling callback functions
  *
  *     (c) 2004, Robert Spalek <robert@ucw.cz>
+ *     (c) 2006 Martin Mares <mj@ucw.cz>
  */
 
 #include "lib/lib.h"
+#include "lib/threads.h"
 
 #include <stdlib.h>
 #include <string.h>
 #include <signal.h>
 
-sh_sighandler_t signal_handler[NSIG];
+static int sig_handler_nest[NSIG];
+static struct sigaction sig_handler_old[NSIG];
 
 static void
 signal_handler_internal(int sig)
 {
-  if (signal_handler[sig])
-  {
-    if (!signal_handler[sig](sig))
-      return;
-  }
-  abort();
+  struct ucwlib_context *ctx = ucwlib_thread_context();
+  if (!ctx->signal_handlers[sig] || ctx->signal_handlers[sig](sig))
+    abort();
 }
 
 void
-handle_signal(int signum, struct sigaction *oldact)
+handle_signal(int signum)
 {
-  struct sigaction act;
-  bzero(&act, sizeof(act));
-  act.sa_handler = signal_handler_internal;
-  act.sa_flags = SA_NODEFER;
-  if (sigaction(signum, &act, oldact) < 0)
-    die("sigaction: %m");
+  ucwlib_lock();
+  if (!sig_handler_nest[signum]++)
+    {
+      struct sigaction act;
+      bzero(&act, sizeof(act));
+      act.sa_handler = signal_handler_internal;
+      act.sa_flags = SA_NODEFER;
+      if (sigaction(signum, &act, &sig_handler_old[signum]) < 0)
+       die("sigaction: %m");
+    }
+  ucwlib_unlock();
 }
 
 void
-unhandle_signal(int signum, struct sigaction *oldact)
+unhandle_signal(int signum)
 {
-  if (sigaction(signum, oldact, NULL) < 0)
-    die("sigaction: %m");
+  ucwlib_lock();
+  ASSERT(sig_handler_nest[signum]);
+  if (!--sig_handler_nest[signum])
+    {
+      if (sigaction(signum, &sig_handler_old[signum], NULL) < 0)
+       die("sigaction: %m");
+    }
+  ucwlib_unlock();
+}
+
+sh_sighandler_t
+set_signal_handler(int signum, sh_sighandler_t new)
+{
+  struct ucwlib_context *ctx = ucwlib_thread_context();
+  if (!ctx->signal_handlers)
+    ctx->signal_handlers = xmalloc_zero(NSIG * sizeof(sh_sighandler_t));
+  sh_sighandler_t old = ctx->signal_handlers[signum];
+  ctx->signal_handlers[signum] = new;
+  return old;
 }

diff --git a/lib/threads.h b/lib/threads.h
index 355902d668a06fd533b8b8dcd94f6aadaa564ea8..9e33109e72eaca794be58fd1e9e8f8ca7bfb18f2 100644 (file)
--- a/lib/threads.h
+++ b/lib/threads.h
@@ -15,6 +15,7 @@
 struct ucwlib_context {
   int temp_counter;                    // Counter for fb-temp.c
   struct asio_queue *io_queue;         // Async I/O queue for fb-direct.c
+  sh_sighandler_t *signal_handlers;    // Signal handlers for sighandler.c
 };
 
 struct ucwlib_context *ucwlib_thread_context(void);