libpthread problem + possible solution

[Peter Edwards]

Hi,

I've a problem when a process uses:
        libpthread
        detached threads
        mixed bound/unbound threads
        suspended threads  (a la pthread_resume_np())

whereby some newly created suspended threads don't get scheduled.
I think I've tracked it down, so if someone could review the
reasoning, I'd be grateful.

Newly launched threads have a "struct pthread" that may be allocated
from a freelist of GCed threads. Apparently, when detached threads
enter the GCed list, they can still have the "active" flag set on
them. Later, this causes problems when this thread is recycled and
resumed, because _thr_setrunnable_unlocked() doesn't add it to a
run queue.

thr_cleanup can be called either from the bound-threads scheduler,
or the unbound scheduler. One callsite clears "active", "needswitchout",
and "lock_switch" to zero before the call. The other callsite just
clears "check_pending". I think these flags are all either bound-thread
or unbound-thread specific, and that there was an unintended
assumption that the thread would remain with the same "boundedness"
after being recycled, which isn't neccessarily the case. (Or another
way - the idea was that there was no need to clear the "active"
flag on a bound thread, as its only used for unbound threads, but
a GCed bound thread might be recycled into an unbound thread)

Given that, it seems correct to clean up the thread the same way
for both cases, and just move that code into thr_cleanup. So, does
the attached patch make sense? I can commit it if someone gives me
the nod. (It definitely fixes my specific problem with threads not
getting scheduled.)
-------------- next part --------------
Index: lib/libpthread/thread/thr_kern.c
===================================================================
RCS file: /net/dyson/export/home/petere/FreeBSD-CVS/src/lib/libpthread/thread/thr_kern.c,v
retrieving revision 1.116.2.1
diff -u -r1.116.2.1 thr_kern.c
--- lib/libpthread/thread/thr_kern.c	16 Mar 2006 23:29:07 -0000	1.116.2.1
+++ lib/libpthread/thread/thr_kern.c	15 Dec 2006 17:48:20 -0000
@@ -764,7 +764,6 @@
 		break;
 
 	case PS_DEAD:
-		curthread->check_pending = 0;
 		/* Unlock the scheduling queue and exit the KSE and thread. */
 		thr_cleanup(curkse, curthread);
 		KSE_SCHED_UNLOCK(curkse, curkse->k_kseg);
@@ -1150,6 +1149,11 @@
 	struct kse_mailbox *kmbx = NULL;
 	int sys_scope;
 
+	thread->active = 0;
+	thread->need_switchout = 0;
+	thread->lock_switch = 0;
+	thread->check_pending = 0;
+
 	if ((joiner = thread->joiner) != NULL) {
 		/* Joinee scheduler lock held; joiner won't leave. */
 		if (joiner->kseg == curkse->k_kseg) {
@@ -1717,9 +1721,6 @@
 			 * stack.  It is safe to do garbage collecting
 			 * here.
 			 */
-			thread->active = 0;
-			thread->need_switchout = 0;
-			thread->lock_switch = 0;
 			thr_cleanup(kse, thread);
 			return;
 			break;