Mask signals in thread creation and destruction to avoid a segfault

If our signal handler gets called before a thread got a chance to run
PERL_SET_CONTEXT(), the call to get the thread-specific interpreter will
fail, and we'll end up with aTHX == NULL.  Prevent this from happening
by blocking most signals right before creating the new thread.  This
way, the new thread starts out with most signals blocked, and it
unblocks them when it's ready to handle them.

This required saving the original signal mask somewhere - I put it in
the thread struct.

In several places, PERL_SET_CONTEXT() is called before the target perl
interpreter struct has been fully initialized, so this patch adds code
to block signals around those blocks of code.

diff --git a/ext/threads/threads.xs b/ext/threads/threads.xs
old mode 100644 (file)
new mode 100755 (executable)
index a15f7ec..9574653
--- a/ext/threads/threads.xs
+++ b/ext/threads/threads.xs
@@ -75,6 +75,7 @@ typedef struct _ithread {
     IV stack_size;
     SV *err;                    /* Error from abnormally terminated thread */
     char *err_class;            /* Error object's classname if applicable */
+    sigset_t initial_sigmask;   /* Thread wakes up with signals blocked */
 } ithread;
 
 
@@ -114,6 +115,44 @@ typedef struct {
 
 #define MY_POOL (*my_poolp)
 
+/* Block most signals for calling thread, setting the old signal mask to
+ * oldmask, if it is not NULL */
+STATIC int
+S_block_most_signals(sigset_t *oldmask)
+{
+    sigset_t newmask;
+
+    sigfillset(&newmask);
+    /* Don't block certain "important" signals (stolen from mg.c) */
+#ifdef SIGILL
+    sigdelset(&newmask, SIGILL);
+#endif
+#ifdef SIGBUS
+    sigdelset(&newmask, SIGBUS);
+#endif
+#ifdef SIGSEGV
+    sigdelset(&newmask, SIGSEGV);
+#endif
+
+#ifdef WIN32
+    /* XXX: How to do this on win32? */
+    return 0;
+#else
+    return pthread_sigmask(SIG_BLOCK, &newmask, oldmask);
+#endif /* WIN32 */
+}
+
+/* Set the signal mask for this thread to newmask */
+STATIC int
+S_set_sigmask(sigset_t *newmask)
+{
+#ifdef WIN32
+    /* XXX: How to do this on win32? */
+    return 0;
+#else
+    return pthread_sigmask(SIG_SETMASK, newmask, NULL);
+#endif /* WIN32 */
+}
 
 /* Used by Perl interpreter for thread context switching */
 STATIC void
@@ -142,12 +181,19 @@ STATIC void
 S_ithread_clear(pTHX_ ithread *thread)
 {
     PerlInterpreter *interp;
+    sigset_t origmask;
 
     assert(((thread->state & PERL_ITHR_FINISHED) &&
             (thread->state & PERL_ITHR_UNCALLABLE))
                 ||
            (thread->state & PERL_ITHR_NONVIABLE));
 
+    /* We temporarily set the interpreter context to the interpreter being
+     * destroyed.  It's in no condition to handle signals while it's being
+     * taken apart.
+     */
+    S_block_most_signals(&origmask);
+
     interp = thread->interp;
     if (interp) {
         dTHXa(interp);
@@ -169,6 +215,7 @@ S_ithread_clear(pTHX_ ithread *thread)
     }
 
     PERL_SET_CONTEXT(aTHX);
+    S_set_sigmask(&origmask);
 }
 
 
@@ -415,6 +462,11 @@ S_ithread_run(void * arg)
     PERL_SET_CONTEXT(thread->interp);
     S_ithread_set(aTHX_ thread);
 
+    /* Thread starts with most signals blocked - restore the signal mask from
+     * the ithread struct.
+     */
+    S_set_sigmask(&thread->initial_sigmask);
+
     PL_perl_destruct_level = 2;
 
     {
@@ -448,6 +500,12 @@ S_ithread_run(void * arg)
         }
         JMPENV_POP;
 
+        /* The interpreter is finished, so this thread can stop receiving
+         * signals.  This way, our signal handler doesn't get called in the
+         * middle of our parent thread calling perl_destruct()...
+         */
+        S_block_most_signals(NULL);
+
         /* Remove args from stack and put back in params array */
         SPAGAIN;
         for (ii=len-1; ii >= 0; ii--) {
@@ -660,6 +718,25 @@ S_ithread_create(
     PL_srand_called = FALSE;   /* Set it to false so we can detect if it gets
                                   set during the clone */
 
+    /* perl_clone() will leave us the new interpreter's context.  This poses
+     * two problems for our signal handler.  First, it sets the new context
+     * before the new interpreter struct is fully initialized, so our signal
+     * handler might find bogus data in the interpreter struct it gets.
+     * Second, even if the interpreter is initialized before a signal comes in,
+     * we would like to avoid that interpreter receiving notifications for
+     * signals (especially when they ought to be for the one running in this
+     * thread), until it is running in its own thread.  Another problem is that
+     * the new thread will not have set the context until some time after it
+     * has started, so it won't be safe for our signal handler to run until
+     * that time.
+     *
+     * So we block most signals here, so the new thread will inherit the signal
+     * mask, and unblock them right after the thread creation.  The original
+     * mask is saved in the thread struct so that the new thread can restore
+     * the original mask.
+     */
+    S_block_most_signals(&thread->initial_sigmask);
+
 #ifdef WIN32
     thread->interp = perl_clone(aTHX, CLONEf_KEEP_PTR_TABLE | CLONEf_CLONE_HOST);
 #else
@@ -774,6 +851,11 @@ S_ithread_create(
 #  endif
         }
 
+    /* Now it's safe to accept signals, since we're in our own interpreter's
+     * context and we have created the thread.
+     */
+    S_set_sigmask(&thread->initial_sigmask);
+
 #  ifdef _POSIX_THREAD_ATTR_STACKSIZE
         /* Try to get thread's actual stack size */
         {