Fix 361615 - Inconsistent termination for multithreaded process terminated by signal

Test program by earl_chew

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

diff --git a/NEWS b/NEWS
index 6254946dfbcacfbed2d75b6a875c633d42083983..5231a5c1b2475ba9f7b7f4e8169f3dc95a533a5c 100644 (file)
--- a/NEWS
+++ b/NEWS
@@ -143,6 +143,7 @@ where XXXXXX is the bug number as listed below.
 361226  s390x: risbgn (EC59) not implemented
 361253  [s390x] ex_clone.c:42: undefined reference to `pthread_create'
 361354  ppc64[le]: wire up separate socketcalls system calls
+361615  Inconsistent termination for multithreaded process terminated by signal
 361926  Unhandled Solaris syscall: sysfs(84)
 362009  Valgrind dumps core on unimplemented functionality before threads are created
 362329  Valgrind does not support the IBM POWER ISA 3.0 instructions, part 3/5

diff --git a/coregrind/m_main.c b/coregrind/m_main.c
index 80f347e6c172fd1733268c5e1ec7825287657949..baf2ebf38cb52ae0acad8e6e9b98600c35848936 100644 (file)
--- a/coregrind/m_main.c
+++ b/coregrind/m_main.c
@@ -2705,7 +2705,11 @@ void shutdown_actions_NORETURN( ThreadId tid,
       sys_exit, do likewise; if the (last) thread stopped due to a fatal
       signal, terminate the entire system with that same fatal signal. */
    VG_(debugLog)(1, "core_os", 
-                    "VG_(terminate_NORETURN)(tid=%u)\n", tid);
+                 "VG_(terminate_NORETURN)(tid=%u) schedretcode %s"
+                 " os_state.exit_code %d fatalsig %d\n",
+                 tid, VG_(name_of_VgSchedReturnCode)(tids_schedretcode),
+                 VG_(threads)[tid].os_state.exitcode, 
+                 VG_(threads)[tid].os_state.fatalsig);
 
    switch (tids_schedretcode) {
    case VgSrc_ExitThread:  /* the normal way out (Linux, Solaris) */

diff --git a/coregrind/m_scheduler/scheduler.c b/coregrind/m_scheduler/scheduler.c
index 9aa854db6e2117bfec6640aff573d55923784a83..727275c29a4dee562eadcbfcfad340bb616983e4 100644 (file)
--- a/coregrind/m_scheduler/scheduler.c
+++ b/coregrind/m_scheduler/scheduler.c
@@ -1653,11 +1653,6 @@ VgSchedReturnCode VG_(scheduler) ( ThreadId tid )
 }
 
 
-/* 
-   This causes all threads to forceably exit.  They aren't actually
-   dead by the time this returns; you need to call
-   VG_(reap_threads)() to wait for them.
- */
 void VG_(nuke_all_threads_except) ( ThreadId me, VgSchedReturnCode src )
 {
    ThreadId tid;

diff --git a/coregrind/m_signals.c b/coregrind/m_signals.c
index 9146d0c5a714dcf36f37f65e2dc982925297c89d..168b681e97447791a9c38bdf30ce54b4ab2ab503 100644 (file)
--- a/coregrind/m_signals.c
+++ b/coregrind/m_signals.c
@@ -1654,8 +1654,8 @@ static Bool is_signal_from_kernel(ThreadId tid, int signum, int si_code)
 
 /* 
    Perform the default action of a signal.  If the signal is fatal, it
-   marks all threads as needing to exit, but it doesn't actually kill
-   the process or thread.
+   terminates all other threads, but it doesn't actually kill
+   the process and calling thread.
 
    If we're not being quiet, then print out some more detail about
    fatal signals (esp. core dumping signals).
@@ -1933,12 +1933,13 @@ static void default_action(const vki_siginfo_t *info, ThreadId tid)
       VG_(setrlimit)(VKI_RLIMIT_CORE, &zero);
    }
 
-   /* stash fatal signal in main thread */
    // what's this for?
    //VG_(threads)[VG_(master_tid)].os_state.fatalsig = sigNo;
 
-   /* everyone dies */
+   /* everyone but tid dies */
    VG_(nuke_all_threads_except)(tid, VgSrc_FatalSig);
+   VG_(reap_threads)(tid);
+   /* stash fatal signal in this thread */
    VG_(threads)[tid].exitreason = VgSrc_FatalSig;
    VG_(threads)[tid].os_state.fatalsig = sigNo;
 }

diff --git a/coregrind/pub_core_scheduler.h b/coregrind/pub_core_scheduler.h
index 08519509584316c0d506d4f52f9e138699c03933..aa9610e43df032551cbe85fb7f1188956476871d 100644 (file)
--- a/coregrind/pub_core_scheduler.h
+++ b/coregrind/pub_core_scheduler.h
@@ -51,7 +51,9 @@ extern void VG_(exit_thread)(ThreadId tid);
    If it isn't blocked in a syscall, has no effect on the thread. */
 extern void VG_(get_thread_out_of_syscall)(ThreadId tid);
 
-/* Nuke all threads except tid. */
+/* This causes all threads except tid to forceably exit.  They aren't actually
+   dead by the time this returns; you need to call
+   VG_(reap_threads)() to wait for them. */
 extern void VG_(nuke_all_threads_except) ( ThreadId me,
                                            VgSchedReturnCode reason );
 

diff --git a/none/tests/Makefile.am b/none/tests/Makefile.am
index 7059e67090b8004f75ea9ba4215a4b185472f87b..11bc3fa958e8ba615db1f988910226f067d9d340 100644 (file)
--- a/none/tests/Makefile.am
+++ b/none/tests/Makefile.am
@@ -167,6 +167,7 @@ EXTRA_DIST = \
        pth_rwlock.stderr.exp pth_rwlock.vgtest \
        pth_stackalign.stderr.exp \
        pth_stackalign.stdout.exp pth_stackalign.vgtest \
+       pth_term_signal.stderr.exp pth_term_signal.vgtest \
        rcrl.stderr.exp rcrl.stdout.exp rcrl.vgtest \
        readline1.stderr.exp readline1.stdout.exp \
        readline1.vgtest \
@@ -224,7 +225,7 @@ check_PROGRAMS = \
        pselect_sigmask_null \
        pth_atfork1 pth_blockedsig pth_cancel1 pth_cancel2 pth_cvsimple \
        pth_empty pth_exit pth_exit2 pth_mutexspeed pth_once pth_rwlock \
-       pth_stackalign \
+       pth_stackalign pth_term_signal\
        rcrl readline1 \
        require-text-symbol \
        res_search resolv \
@@ -315,6 +316,7 @@ if VGCONF_OS_IS_SOLARIS
 pth_rwlock_CFLAGS      += --std=c99
 endif
 pth_stackalign_LDADD   = -lpthread
+pth_term_signal_LDADD  = -lpthread
 res_search_LDADD        = -lresolv -lpthread
 resolv_CFLAGS          = $(AM_CFLAGS)
 resolv_LDADD            = -lresolv -lpthread

diff --git a/none/tests/pth_term_signal.c b/none/tests/pth_term_signal.c
new file mode 100644 (file)
index 0000000..9d8f05c
--- /dev/null
+++ b/none/tests/pth_term_signal.c
@@ -0,0 +1,91 @@
+#include <stdio.h>
+#include <unistd.h>
+#include <signal.h>
+#include <pthread.h>
+#include <assert.h>
+
+#include <sys/wait.h>
+
+void *
+slavethread(void *arg)
+{
+    sigset_t sigmask;
+
+    if (sigfillset(&sigmask))
+    {
+        fprintf(stderr, "Error line %u\n", __LINE__);
+        _exit(255);
+    }
+
+    if (pthread_sigmask(SIG_UNBLOCK, &sigmask, 0))
+    {
+        fprintf(stderr, "Error line %u\n", __LINE__);
+        _exit(255);
+    }
+
+    while (1)
+        sleep(1);
+}
+
+void
+childprocess()
+{
+    pthread_t slave;
+
+    if (pthread_create(&slave, 0, &slavethread, 0))
+    {
+        fprintf(stderr, "Error line %u\n", __LINE__);
+        _exit(255);
+    }
+
+    while (1)
+        sleep(1);
+}
+
+int main(int argc, char **argv)
+{
+    sigset_t sigmask;
+
+    if (sigfillset(&sigmask))
+    {
+        fprintf(stderr, "Error line %u\n", __LINE__);
+        return 255;
+    }
+
+    if (pthread_sigmask(SIG_BLOCK, &sigmask, 0))
+    {
+        fprintf(stderr, "Error line %u\n", __LINE__);
+        return 255;
+    }
+
+    int childpid = fork();
+
+    if (-1 == childpid)
+    {
+        fprintf(stderr, "Error line %u\n", __LINE__);
+        return 255;
+    }
+
+    if ( ! childpid)
+        childprocess();
+
+    if (kill(childpid, SIGTERM))
+    {
+        fprintf(stderr, "Error line %u\n", __LINE__);
+        return 255;
+    }
+
+    int status;
+    if (childpid != waitpid(childpid, &status, 0))
+    {
+        fprintf(stderr, "Error line %u\n", __LINE__);
+        return 255;
+    }
+
+    assert(WIFSIGNALED(status));
+
+    fprintf(stderr, "Signal %d\n", WTERMSIG(status));
+    assert(WTERMSIG(status) == SIGTERM);
+
+    return 0;
+}

diff --git a/none/tests/pth_term_signal.stderr.exp b/none/tests/pth_term_signal.stderr.exp
new file mode 100644 (file)
index 0000000..272cf31
--- /dev/null
+++ b/none/tests/pth_term_signal.stderr.exp
@@ -0,0 +1 @@
+Signal 15

diff --git a/none/tests/pth_term_signal.vgtest b/none/tests/pth_term_signal.vgtest
new file mode 100644 (file)
index 0000000..9591605
--- /dev/null
+++ b/none/tests/pth_term_signal.vgtest
@@ -0,0 +1,2 @@
+prog: pth_term_signal
+vgopts: -q