]> mj.ucw.cz Git - libucw.git/commitdiff
Fixed thread safety of signal handling.
authorMartin Mares <mj@ucw.cz>
Mon, 11 Dec 2006 22:45:56 +0000 (23:45 +0100)
committerMartin Mares <mj@ucw.cz>
Mon, 11 Dec 2006 22:45:56 +0000 (23:45 +0100)
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.

lib/lib.h
lib/lizard-safe.c
lib/sighandler.c

index 43135c6cc27f5409ece71fd5cd4c3a7376bbc6c1..4a6b9d81e3620fd0ddb0908e6d46e421fb7caf55 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 */
 
index 84e52da18e54dd5f87f84fba75dd12d4d186671e..7553b45944e04269a73b6dfbb1be0ea48853829b 100644 (file)
@@ -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", (uns)(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;
 }
index 4d991d8562f4b7cdfe70a44d7f8978b1a5037d4f..4991f9a3379b080fb0681cac303c24abdf138125 100644 (file)
@@ -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;
 }