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

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

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) */

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;

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;
 }

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 );
 

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

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;
+}

none/tests/pth_term_signal.stderr.exp

@@ -0,0 +1 @@
+Signal 15

none/tests/pth_term_signal.vgtest

@@ -0,0 +1,2 @@
+prog: pth_term_signal
+vgopts: -q