Fix bug in signal handling in threads shown by Peter Gober:

A signal *specifically directed* to one thread cannot be used
to fulfill a sigwait() request by some other thread, reasonably
enough.

git-svn-id: svn://svn.valgrind.org/valgrind/trunk@374

diff --git a/coregrind/vg_signals.c b/coregrind/vg_signals.c
index 7a8c737e0412098c2ca8b3e4c85662212b56926a..2f2fe148535fbf60b12983e119559f28f150432a 100644 (file)
--- a/coregrind/vg_signals.c
+++ b/coregrind/vg_signals.c
@@ -1059,8 +1059,15 @@ Bool VG_(deliver_signals) ( void )
          If so just give to one of them and have done. */
       for (tid = 1; tid < VG_N_THREADS; tid++) {
          tst = & VG_(threads)[tid];
+         /* Is tid waiting for a signal?  If not, ignore. */
          if (tst->status != VgTs_WaitSIG)
             continue;
+        /* Is the signal directed at a specific thread other than
+            this one?  If yes, ignore. */
+         if (vg_dcss.dcss_destthread[sigNo] != VG_INVALID_THREADID
+             && vg_dcss.dcss_destthread[sigNo] != tid)
+            continue;
+         /* Is tid waiting for the signal?  If not, ignore. */
          if (VG_(ksigismember)(&(tst->sigs_waited_for), sigNo))
             break;
       }

diff --git a/tests/Makefile.am b/tests/Makefile.am
index 8770a85c20eced3f647b114b6e8926a9756b1d70..d68b430d7594dafc67958ddb577f38a4f12852de 100644 (file)
--- a/tests/Makefile.am
+++ b/tests/Makefile.am
@@ -30,4 +30,4 @@ EXTRA_DIST = \
        pth_once.c weirdioctl.c pth_signal1.c pth_signal2.c \
        discard.c pth_semaphore1.c new_override.cpp pth_yield.c \
        sigaltstack.c erringfds.c sigwait_all.c \
-       pth_cancel1.c pth_cancel2.c
+       pth_cancel1.c pth_cancel2.c pth_signal_gober.c

diff --git a/tests/pth_signal_gober.c b/tests/pth_signal_gober.c
new file mode 100644 (file)
index 0000000..7e1f124
--- /dev/null
+++ b/tests/pth_signal_gober.c
@@ -0,0 +1,65 @@
+#include <pthread.h>
+#include <signal.h>
+#include <stdio.h>
+#include <unistd.h>
+
+/* thread function from Galaxy Communicator library */
+static void *__Gal_SignalHandler(void *arg)
+{
+  sigset_t set;
+  int sig;
+  int res;
+
+  sigfillset(&set);
+
+  while(1) {
+    res = sigwait(&set, &sig);
+    printf("Received signal number %d\n", sig);
+  }
+}
+
+/* function from my code */
+static void signal_handler(int i)
+{
+  // nop
+}
+
+/* function from my code */
+static void *serve(void *arg)
+{
+  sigset_t sig;
+  sigemptyset(&sig);
+  sigaddset(&sig, SIGUSR1);
+  pthread_sigmask(SIG_UNBLOCK, &sig, NULL);
+
+  for(;;) {
+    /* somewhere in here, deeply buried within libcapi20, is a select(),
+       that I am interrupting by sending SIGUSR1 to this thread */
+  }
+}
+
+int main( void )
+{
+  pthread_t sig_thread, serve_thread;
+  pthread_attr_t sig_attr;
+  struct sigaction sigact;
+
+  /* from Galaxy Communicator library */
+  pthread_attr_init(&sig_attr);
+  pthread_attr_setdetachstate(&sig_attr, PTHREAD_CREATE_DETACHED);
+  pthread_create(&sig_thread, &sig_attr, __Gal_SignalHandler, NULL);
+
+  /* from my code */
+  sigemptyset(&sigact.sa_mask);
+  sigact.sa_handler = signal_handler;
+  sigact.sa_flags = 0;
+  sigaction(SIGUSR1, &sigact, NULL);
+  pthread_create(&serve_thread, NULL, serve, NULL);
+
+  /* happens within my code */
+  for(;;) {
+    pthread_kill(serve_thread, SIGUSR1);
+    sleep(1);
+  }
+  return 0;
+}

diff --git a/vg_signals.c b/vg_signals.c
index 7a8c737e0412098c2ca8b3e4c85662212b56926a..2f2fe148535fbf60b12983e119559f28f150432a 100644 (file)
--- a/vg_signals.c
+++ b/vg_signals.c
@@ -1059,8 +1059,15 @@ Bool VG_(deliver_signals) ( void )
          If so just give to one of them and have done. */
       for (tid = 1; tid < VG_N_THREADS; tid++) {
          tst = & VG_(threads)[tid];
+         /* Is tid waiting for a signal?  If not, ignore. */
          if (tst->status != VgTs_WaitSIG)
             continue;
+        /* Is the signal directed at a specific thread other than
+            this one?  If yes, ignore. */
+         if (vg_dcss.dcss_destthread[sigNo] != VG_INVALID_THREADID
+             && vg_dcss.dcss_destthread[sigNo] != tid)
+            continue;
+         /* Is tid waiting for the signal?  If not, ignore. */
          if (VG_(ksigismember)(&(tst->sigs_waited_for), sigNo))
             break;
       }