Implementation of an additional state called SIGWAIT (with the previous

one renamed to SIGSUSPEND) to fix sigwait().

Submitted by: Daniel M. Eischen <eischen@vigrid.com>

4 files changed, 116 insertions, 68 deletions

diff --git a/lib/libpthread/thread/thr_kill.c b/lib/libpthread/thread/thr_kill.c
index 98b3a2d..292ba5c 100644
--- a/lib/libpthread/thread/thr_kill.c
+++ b/lib/libpthread/thread/thr_kill.c
@@ -46,18 +46,48 @@ pthread_kill(pthread_t pthread, int sig)
 		/* Invalid signal: */
 		ret = EINVAL;
 
+	/* Ignored signals get dropped on the floor. */
+	else if (_thread_sigact[sig - 1].sa_handler == SIG_IGN)
+		ret = 0;
+
 	/* Find the thread in the list of active threads: */
 	else if ((ret = _find_thread(pthread)) == 0) {
-		if ((pthread->state == PS_SIGWAIT) &&
-		    sigismember(&pthread->sigmask, sig)) {
-			/* Change the state of the thread to run: */
-			PTHREAD_NEW_STATE(pthread,PS_RUNNING);
+		switch (pthread->state) {
+		case PS_SIGSUSPEND:
+			/*
+			 * Only wake up the thread if the signal is unblocked
+			 * and there is a handler installed for the signal.
+			 */
+			if (!sigismember(&pthread->sigmask, sig) &&
+			    _thread_sigact[sig - 1].sa_handler != SIG_DFL) {
+				/* Change the state of the thread to run: */
+				PTHREAD_NEW_STATE(pthread,PS_RUNNING);
+
+				/* Return the signal number: */
+				pthread->signo = sig;
+			}
+			/* Increment the pending signal count: */
+			sigaddset(&pthread->sigpend,sig);
+			break;
+
+		case PS_SIGWAIT:
+			/* Wake up the thread if the signal is blocked. */
+			if (sigismember(pthread->data.sigwait, sig)) {
+				/* Change the state of the thread to run: */
+				PTHREAD_NEW_STATE(pthread,PS_RUNNING);
+
+				/* Return the signal number: */
+				pthread->signo = sig;
+			} else
+				/* Increment the pending signal count. */
+				sigaddset(&pthread->sigpend,sig);
+			break;
 
-			/* Return the signal number: */
-			pthread->signo = sig;
-		} else
+		default:
 			/* Increment the pending signal count: */
 			sigaddset(&pthread->sigpend,sig);
+			break;
+		}
 	}
 
 	/* Return the completion status: */
diff --git a/lib/libpthread/thread/thr_sig.c b/lib/libpthread/thread/thr_sig.c
index d0d5f8b..3e55d65 100644
--- a/lib/libpthread/thread/thr_sig.c
+++ b/lib/libpthread/thread/thr_sig.c
@@ -39,9 +39,7 @@
 #include "pthread_private.h"
 
 /* Static variables: */
-static int		volatile yield_on_unlock_dead	= 0;
 static int		volatile yield_on_unlock_thread	= 0;
-static spinlock_t	thread_dead_lock	= _SPINLOCK_INITIALIZER;
 static spinlock_t	thread_link_list_lock	= _SPINLOCK_INITIALIZER;
 
 /* Lock the thread list: */
@@ -52,14 +50,6 @@ _lock_thread_list()
 	_SPINLOCK(&thread_link_list_lock);
 }
 
-/* Lock the dead thread list: */
-void
-_lock_dead_thread_list()
-{
-	/* Lock the dead thread list: */
-	_SPINLOCK(&thread_dead_lock);
-}
-
 /* Lock the thread list: */
 void
 _unlock_thread_list()
@@ -80,26 +70,6 @@ _unlock_thread_list()
 	}
 }
 
-/* Lock the dead thread list: */
-void
-_unlock_dead_thread_list()
-{
-	/* Unlock the dead thread list: */
-	_SPINUNLOCK(&thread_dead_lock);
-
-	/*
-	 * Check if a scheduler interrupt occurred while the dead
-	 * thread list was locked:
-	 */
-	if (yield_on_unlock_dead) {
-		/* Reset the interrupt flag: */
-		yield_on_unlock_dead = 0;
-
-		/* This thread has overstayed it's welcome: */
-		sched_yield();
-	}
-}
-
 void
 _thread_sig_handler(int sig, int code, struct sigcontext * scp)
 {
@@ -144,18 +114,6 @@ _thread_sig_handler(int sig, int code, struct sigcontext * scp)
 			 */
 			yield_on_unlock_thread = 1;
 
-		/* Check if the scheduler interrupt has come at an
-		 * unfortunate time which one of the threads is
-		 * modifying the dead thread list:
-		 */
-		if (thread_dead_lock.access_lock)
-			/*
-			 * Set a flag so that the thread that has
-			 * the lock yields when it unlocks the
-			 * dead thread list:
-			 */
-			yield_on_unlock_dead = 1;
-
 		/*
 		 * Check if the kernel has not been interrupted while
 		 * executing scheduler code:
@@ -198,7 +156,7 @@ _thread_sig_handler(int sig, int code, struct sigcontext * scp)
 
 		/*
 		 * POSIX says that pending SIGCONT signals are
-		 * discarded when one of there signals occurs.
+		 * discarded when one of these signals occurs.
 		 */
 		if (sig == SIGTSTP || sig == SIGTTIN || sig == SIGTTOU) {
 			/*
@@ -211,6 +169,31 @@ _thread_sig_handler(int sig, int code, struct sigcontext * scp)
 				sigdelset(&pthread->sigpend,SIGCONT);
 		}
 
+		/*
+		 * Enter a loop to process each thread in the linked
+		 * list that is sigwait-ing on a signal.  Since POSIX
+		 * doesn't specify which thread will get the signal
+		 * if there are multiple waiters, we'll give it to the
+		 * first one we find.
+		 */
+		for (pthread = _thread_link_list; pthread != NULL;
+		    pthread = pthread->nxt) {
+			if ((pthread->state == PS_SIGWAIT) &&
+			    sigismember(pthread->data.sigwait, sig)) {
+				/* Change the state of the thread to run: */
+				PTHREAD_NEW_STATE(pthread,PS_RUNNING);
+
+				/* Return the signal number: */
+				pthread->signo = sig;
+
+				/*
+				 * Do not attempt to deliver this signal
+				 * to other threads.
+				 */
+				return;
+			}
+		}
+
 		/* Check if the signal is not being ignored: */
 		if (_thread_sigact[sig - 1].sa_handler != SIG_IGN)
 			/*
@@ -255,6 +238,7 @@ _thread_signal(pthread_t pthread, int sig)
 	case PS_RUNNING:
 	case PS_STATE_MAX:
 	case PS_SIGTHREAD:
+	case PS_SIGWAIT:
 	case PS_SUSPENDED:
 		/* Nothing to do here. */
 		break;
@@ -287,7 +271,6 @@ _thread_signal(pthread_t pthread, int sig)
 	case PS_FDW_WAIT:
 	case PS_SLEEP_WAIT:
 	case PS_SELECT_WAIT:
-	case PS_SIGWAIT:
 		if (sig != SIGCHLD ||
 		    _thread_sigact[sig - 1].sa_handler != SIG_DFL) {
 			/* Flag the operation as interrupted: */
@@ -300,6 +283,21 @@ _thread_signal(pthread_t pthread, int sig)
 			pthread->signo = sig;
 		}
 		break;
+
+	case PS_SIGSUSPEND:
+		/*
+		 * Only wake up the thread if the signal is unblocked
+		 * and there is a handler installed for the signal.
+		 */
+		if (!sigismember(&pthread->sigmask, sig) &&
+		    _thread_sigact[sig - 1].sa_handler != SIG_DFL) {
+			/* Change the state of the thread to run: */
+			PTHREAD_NEW_STATE(pthread,PS_RUNNING);
+
+			/* Return the signal number: */
+			pthread->signo = sig;
+		}
+		break;
 	}
 }
 
diff --git a/lib/libpthread/thread/thr_sigsuspend.c b/lib/libpthread/thread/thr_sigsuspend.c
index f7a7b12..0ae9a4f 100644
--- a/lib/libpthread/thread/thr_sigsuspend.c
+++ b/lib/libpthread/thread/thr_sigsuspend.c
@@ -44,14 +44,14 @@ sigsuspend(const sigset_t * set)
 
 	/* Check if a new signal set was provided by the caller: */
 	if (set != NULL) {
-		/* Save the current sigmal mask: */
+		/* Save the current signal mask: */
 		oset = _thread_run->sigmask;
 
-		/* Combine the caller's mask with the current one: */
-		_thread_run->sigmask |= *set;
+		/* Change the caller's mask: */
+		_thread_run->sigmask = *set;
 
 		/* Wait for a signal: */
-		_thread_kern_sched_state(PS_SIGWAIT, __FILE__, __LINE__);
+		_thread_kern_sched_state(PS_SIGSUSPEND, __FILE__, __LINE__);
 
 		/* Always return an interrupted error: */
 		errno = EINTR;
diff --git a/lib/libpthread/thread/thr_sigwait.c b/lib/libpthread/thread/thr_sigwait.c
index f441fa3..590f9db 100644
--- a/lib/libpthread/thread/thr_sigwait.c
+++ b/lib/libpthread/thread/thr_sigwait.c
@@ -41,7 +41,7 @@ sigwait(const sigset_t * set, int *sig)
 {
 	int		ret = 0;
 	int		i;
-	sigset_t	oset;
+	sigset_t	tempset;
 	struct sigaction act;
 	
 	/*
@@ -60,31 +60,48 @@ sigwait(const sigset_t * set, int *sig)
 	sigdelset(&act.sa_mask, SIGCHLD);
 	sigdelset(&act.sa_mask, SIGINFO);
 
+	/* Check to see if a pending signal is in the wait mask. */
+	if (tempset = (_thread_run->sigpend & act.sa_mask)) {
+		/* Enter a loop to find a pending signal: */
+		for (i = 1; i < NSIG; i++) {
+			if (sigismember (&tempset, i))
+				break;
+		}
+
+		/* Clear the pending signal: */
+		sigdelset(&_thread_run->sigpend,i);
+
+		/* Return the signal number to the caller: */
+		*sig = i;
+
+		return (0);
+	}
+
 	/*
 	 * Enter a loop to find the signals that are SIG_DFL.  For
 	 * these signals we must install a dummy signal handler in
 	 * order for the kernel to pass them in to us.  POSIX says
 	 * that the application must explicitly install a dummy
 	 * handler for signals that are SIG_IGN in order to sigwait
-	 * on them, so we ignore SIG_IGN signals.
+	 * on them.  Note that SIG_IGN signals are left in the
+	 * mask because a subsequent sigaction could enable an
+	 * ignored signal.
 	 */
 	for (i = 1; i < NSIG; i++) {
 		if (sigismember(&act.sa_mask, i)) {
-			if (_thread_sigact[i - 1].sa_handler == SIG_DFL) {
+			if (_thread_sigact[i - 1].sa_handler == SIG_DFL)
 				if (_thread_sys_sigaction(i,&act,NULL) != 0)
 					ret = -1;
-			}
-			else if (_thread_sigact[i - 1].sa_handler == SIG_IGN)
-				sigdelset(&act.sa_mask, i);
 		}
 	}
 	if (ret == 0) {
 
-		/* Save the current signal mask: */
-		oset = _thread_run->sigmask;
-
-		/* Combine the caller's mask with the current one: */
-		_thread_run->sigmask |= act.sa_mask;
+		/*
+		 * Save the wait signal mask.  The wait signal
+		 * mask is independent of the threads signal mask
+		 * and requires separate storage.
+		 */
+		_thread_run->data.sigwait = &act.sa_mask;
 
 		/* Wait for a signal: */
 		_thread_kern_sched_state(PS_SIGWAIT, __FILE__, __LINE__);
@@ -92,8 +109,11 @@ sigwait(const sigset_t * set, int *sig)
 		/* Return the signal number to the caller: */
 		*sig = _thread_run->signo;
 
-		/* Restore the signal mask: */
-		_thread_run->sigmask = oset;
+		/*
+		 * Probably unnecessary, but since it's in a union struct
+		 * we don't know how it could be used in the future.
+		 */
+		_thread_run->data.sigwait = NULL;
 	}
 
 	/* Restore the sigactions: */