Re: Signals and an exiting thread

On Wed, Sep 30, 2009 at 11:02:19AM -0700, Justin Teller wrote:
> We're trying to control one process from another process through
> signals (I know, revolutionary ;-), and we've found that a signal
> occasionally gets lost.  The process we're signaling is
> multi-threaded.  It looks like the signal is lost when the kernel
> decides to post the signal to a thread that is in the process of dying
> (calling pthread_exit, etc).
> 
> Is this expected behavior that we should just handle, or is it a race
> in the kernel that should be/will be/already is fixed?
> 
> It may be that a fix is already in current, and I just haven't found
> it in my searches through the source code (I'm working off of source
> code for an older 8.0 image).  If it is fixed, I'd appreciate a
> pointer to the code that fixes it.

When thread enters the kernel last time to be killed, it is very much
bad idea to allow it to return to usermode to handle directed signal.
And, there would always be window between entering the kernel and
marking the thread as exiting.

Moving the thread-directed signals back to the process queue is hard
and there is no way to distinguish which signals were sent to process
or to the thread.

Possibly, we could clear the thread signal mask while still in user mode.
I think it would still leave a very narrow window when a process
signal could be directed to the dying thread and not be delivered to
usermode; this happens when signal is generated while sigsetmask already
entered the kernel, but did not changed the mask yet. This is worked
around by rechecking the pending signals after setting the block mask
and releasing it if needed.

Consider the patch as experimental.

diff --git a/lib/libthr/thread/thr_exit.c b/lib/libthr/thread/thr_exit.c
index 33a2451..f0339c5 100644
--- a/lib/libthr/thread/thr_exit.c
+++ b/lib/libthr/thread/thr_exit.c
_at__at_ -31,6 +31,7 _at__at_
 
 #include "namespace.h"
 #include <errno.h>
+#include <signal.h>
 #include <stdio.h>
 #include <stdlib.h>
 #include <pthread.h>
_at__at_ -69,6 +70,8 _at__at_ void
 _pthread_exit(void *status)
 {
 	struct pthread *curthread = _get_curthread();
+	sigset_t block_sig, old_sig, pend_sig;
+	int i;
 
 	/* Check if this thread is already in the process of exiting: */
 	if (curthread->cancelling) {
_at__at_ -108,6 +111,30 _at__at_ _pthread_exit(void *status)
 	}
 	THREAD_LIST_UNLOCK(curthread);
 
+	/*
+	 * From now on, hint the kernel to not direct signals for
+	 * delivery to this thread.
+	 */
+	sigfillset(&block_sig);
+	for (;;) {
+		_pthread_sigmask(SIG_SETMASK, &block_sig, &old_sig);
+		/*
+		 * If the signal was directed to the thread before
+		 * thread sigmask was set to blocked, try to handle
+		 * the signal.
+		 */
+		sigpending(&pend_sig);
+		for (i = 0; i < _SIG_MAXSIG; i++) {
+			if (!sigismember(&old_sig, i) &&
+			    sigismember(&pend_sig, i)) {
+				_pthread_sigmask(SIG_SETMASK, &old_sig, NULL);
+				break;
+			}
+		}
+		if (i == _SIG_MAXSIG)
+			break;
+	}
+
 	/* Tell malloc that the thread is exiting. */
 	_malloc_thread_cleanup();