Segmentation fault in Perl_csighandler when receiving SIGCHLD during thread creation #9570
Comments
From mail@pascalhofmann.deI'have a program which uses threads and has a custom signal handler for Program received signal SIGSEGV, Segmentation fault. #11 0x080b22e9 in Perl_runops_standard () #12 0x080b0750 in perl_run () #13 0x08063ebd in main () Program received signal SIGSEGV, Segmentation fault. #73 0x080b22e9 in Perl_runops_standard () #74 0x080b0750 in perl_run () #75 0x08063ebd in main () Flags: category= severity= Site configuration information for perl 5.10.0: Configured by Debian Project at Thu Jul 24 09:01:44 UTC 2008. Summary of my perl5 (revision 5 version 10 subversion 0) configuration: Locally applied patches: @INC for perl 5.10.0: Environment for perl 5.10.0: PATH=/home/pascal/bin:/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin:/usr/games |
From john.wright@hp.comOn Fri Nov 21 00:39:51 2008, mail@pascalhofmann.de wrote:
This bug lies in the fact that Perl_csighandler() relies on the I have attached a reproducer, crash-with-threads.pl, based on one 1. Crash when trying to do "PL_psig_pend[sig]++;". This happens It can also get a signal while it's tearing down a thread. I've My solution here is to wrap the potentially dangerous sections 2. Crashing when accessing PL_signals (which expands to My solution here is to block signals in the calling thread before I also ran into a related bug, in the non-threaded case: If Perl is in My solution to this is to de-register the signal handler before I have attached, as well as the reproducers above, two patches which fix |
From john.wright@hp.com0002-Mask-signals-in-thread-creation-and-destruction-to-a.patchFrom 4c0ed7b7d0b87ebd1cd194a5232ea8b2813edaec Mon Sep 17 00:00:00 2001
From: John Wright <john.wright@hp.com>
Date: Wed, 6 May 2009 15:48:12 -0600
Subject: [PATCH 2/2] 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.
---
ext/threads/threads.xs | 82 ++++++++++++++++++++++++++++++++++++++++++++++++
1 files changed, 82 insertions(+), 0 deletions(-)
mode change 100644 => 100755 ext/threads/threads.xs
diff --git a/ext/threads/threads.xs b/ext/threads/threads.xs
old mode 100644
new mode 100755
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 */
{
--
1.5.6.5
|
From john.wright@hp.com0001-main-Unregister-signal-handler-before-destroying-my.patchFrom a9450face7394716acd9ca8403d0f50e16b60b0a Mon Sep 17 00:00:00 2001
From: John Wright <john@johnwright.org>
Date: Wed, 6 May 2009 00:47:15 -0600
Subject: [PATCH 1/2] main: Unregister signal handler before destroying my_perl
If the signal handler runs after perl_destruct() has been called, it
will get an invalid (or NULL) my_perl when it asks for the
thread-specific interpreter struct. This patch resets the signal
handler for any signal previously handled by PL_csighandlerp to SIG_DFL
before calling perl_destruct().
---
miniperlmain.c | 9 ++++++++-
1 files changed, 8 insertions(+), 1 deletions(-)
diff --git a/miniperlmain.c b/miniperlmain.c
index f60a3e0..f2302c2 100644
--- a/miniperlmain.c
+++ b/miniperlmain.c
@@ -67,7 +67,7 @@ main(int argc, char **argv, char **env)
#endif
{
dVAR;
- int exitstatus;
+ int exitstatus, i;
#ifdef PERL_GLOBAL_STRUCT
struct perl_vars *plvarsp = init_global_struct();
# ifdef PERL_GLOBAL_STRUCT_PRIVATE
@@ -116,6 +116,13 @@ main(int argc, char **argv, char **env)
if (!exitstatus)
perl_run(my_perl);
+ /* Unregister our signal handler before destroying my_perl */
+ for (i = 0; PL_sig_name[i]; i++) {
+ if (rsignal_state(PL_sig_num[i]) == (Sighandler_t) PL_csighandlerp) {
+ rsignal(PL_sig_num[i], (Sighandler_t) SIG_DFL);
+ }
+ }
+
exitstatus = perl_destruct(my_perl);
perl_free(my_perl);
--
1.5.6.5
|
|
The RT System itself - Status changed from 'new' to 'open' |
From @rgsFixed by those patches : commit b762d86 Mask signals in thread creation and destruction to avoid a segfault ext/threads/threads.xs | 82 commit 01be072 main: Unregister signal handler before destroying my_perl |
|
@rgs - Status changed from 'open' to 'resolved' |
Migrated from rt.perl.org#60724 (status was 'resolved')
Searchable as RT60724$
The text was updated successfully, but these errors were encountered: