Skip site navigation (1)Skip section navigation (2)
Date:      Sat, 10 Nov 2001 10:09:14 -0500 (EST)
From:      Daniel Eischen <eischen@pcnet1.pcnet.com>
To:        rittle@labs.mot.com
Cc:        freebsd-hackers@FreeBSD.ORG, current@FreeBSD.ORG
Subject:   Re: Report on FreeBSD 4.4 pthread implementation verses boehm-gc
Message-ID:  <Pine.SUN.3.91.1011110095953.20896A-100000@pcnet1.pcnet.com>
In-Reply-To: <200111090250.fA92o7h55180@latour.rsch.comm.mot.com>

next in thread | previous in thread | raw e-mail | index | archive | help
[ Followups to -current ]

On Thu, 8 Nov 2001, Loren James Rittle wrote:

> Hello all,
> 
> I have ported the most recent version of boehm-gc (6.1-alpha) to
> FreeBSD/i386 under the auspice of the gcc project (it will be in Hans'
> 6.1 release and it is on the gcc mainline).  I got one notable thing
> fully configured beyond what is in the ports tree (which is based on
> 6.0): threaded GC is now supported.  However, this work has uncovered
> either a rare race condition in the 4.X pthread implementation (also
> seen on a current 5.0 system) or a bad assumption in the GC signal
> code (abstracted below).  Either way, the result seen is an undetected
> deadlock.  With the following new assertion, I can at least force the
> condition to be detectable in many cases where it would have locked up.
> 
> Two questions come to mind: Is there any condition under which my new
> assumption should not be true?  Is there any obvious mistake that a
> threaded application can make (perhaps related to its signal use) that
> could cause the new assumption to ever be violated?
> 
> I have also seen what I thought was a less important issue, but I now
> see that it is probably related.  After reviewing the FreeBSD uthread
> source code, the issue appears to be a race between the pthread_exit()
> code running in one thread and the pthread_join() code running in
> another thread in conjunction with a sigsuspend() call occurring on a
> signal handler of that second thread.  Under some conditions, an
> errant EINTR would be returned to the pthread_join() caller instead of
> the exit code from the terminated thread.  Under other timing
> conditions, you get the deadlock spotted with the above new assertion.

Try the following patch; this is to -current, you'll have to massage
it a bit for -stable (Hint: s/curthread/_thread_run/ in -stable).

-- 
Dan Eischen

Index: uthread/pthread_private.h
===================================================================
RCS file: /opt/d/CVS/src/lib/libc_r/uthread/pthread_private.h,v
retrieving revision 1.63
diff -u -r1.63 pthread_private.h
--- uthread/pthread_private.h	26 Oct 2001 21:19:22 -0000	1.63
+++ uthread/pthread_private.h	10 Nov 2001 14:35:48 -0000
@@ -601,6 +601,11 @@
 	/* XXX - What about thread->timeout and/or thread->error? */
 };
 
+struct join_status {
+	struct pthread	*thread;
+	int		ret;
+	int		error;
+};
 
 /*
  * Normally thread contexts are stored as jmp_bufs via 
_setjmp()/_longjmp(),
@@ -757,8 +762,12 @@
 	 */
 	int	error;
 
-	/* Pointer to a thread that is waiting to join (NULL if no joiner). */
-	struct pthread *joiner;
+	/*
+	 * The joiner is the thread that is joining to this thread.  The
+	 * join status keeps track of a join operation to another thread.
+	 */
+	struct pthread		*joiner;
+	struct join_status	join_status;
 
 	/*
 	 * The current thread can belong to only one scheduling queue at
Index: uthread/uthread_exit.c
===================================================================
RCS file: /opt/d/CVS/src/lib/libc_r/uthread/uthread_exit.c,v
retrieving revision 1.23
diff -u -r1.23 uthread_exit.c
--- uthread/uthread_exit.c	20 May 2001 23:08:32 -0000	1.23
+++ uthread/uthread_exit.c	10 Nov 2001 14:36:50 -0000
@@ -220,8 +220,9 @@
 		}
 
 		/* Set the return value for the joining thread: */
-		pthread->ret = curthread->ret;
-		pthread->error = 0;
+		pthread->join_status.ret = curthread->ret;
+		pthread->join_status.error = 0;
+		pthread->join_status.thread = NULL;
 
 		/* Make this thread collectable by the garbage collector. */
 		PTHREAD_ASSERT(((curthread->attr.flags & PTHREAD_DETACHED) ==
Index: uthread/uthread_join.c
===================================================================
RCS file: /opt/d/CVS/src/lib/libc_r/uthread/uthread_join.c,v
retrieving revision 1.19
diff -u -r1.19 uthread_join.c
--- uthread/uthread_join.c	16 Aug 2001 06:31:32 -0000	1.19
+++ uthread/uthread_join.c	10 Nov 2001 14:36:14 -0000
@@ -122,18 +122,20 @@
 		pthread->joiner = curthread;
 
 		/* Keep track of which thread we're joining to: */
-		curthread->data.thread = pthread;
+		curthread->join_status.thread = pthread;
 
-		/* Schedule the next thread: */
-		_thread_kern_sched_state(PS_JOIN, __FILE__, __LINE__);
+		while (curthread->join_status.thread == pthread) {
+			/* Schedule the next thread: */
+			_thread_kern_sched_state(PS_JOIN, __FILE__, __LINE__);
+		}
 
 		/*
 		 * The thread return value and error are set by the thread we're
 		 * joining to when it exits or detaches:
 		 */
-		ret = curthread->error;
+		ret = curthread->join_status.error;
 		if ((ret == 0) && (thread_return != NULL))
-			*thread_return = curthread->ret;
+			*thread_return = curthread->join_status.ret;
 	} else {
 		/*
 		 * The thread exited (is dead) without being detached, and no
Index: uthread/uthread_sig.c
===================================================================
RCS file: /opt/d/CVS/src/lib/libc_r/uthread/uthread_sig.c,v
retrieving revision 1.38
diff -u -r1.38 uthread_sig.c
--- uthread/uthread_sig.c	29 Jun 2001 17:09:07 -0000	1.38
+++ uthread/uthread_sig.c	10 Nov 2001 14:28:09 -0000
@@ -671,7 +671,6 @@
 	 * signal handler to run:
 	 */
 	case PS_COND_WAIT:
-	case PS_JOIN:
 	case PS_MUTEX_WAIT:
 		/*
 		 * Remove the thread from the wait queue.  It will
@@ -679,6 +678,17 @@
 		 * handlers have been invoked.
 		 */
 		PTHREAD_WAITQ_REMOVE(pthread);
+		break;
+
+	case PS_JOIN:
+		/*
+		 * Remove the thread from the wait queue.  It will
+		 * be added back to the wait queue once all signal
+		 * handlers have been invoked.
+		 */
+		PTHREAD_WAITQ_REMOVE(pthread);
+		/* Make the thread runnable: */
+		PTHREAD_SET_STATE(pthread, PS_RUNNING);
 		break;
 
 	/*


To Unsubscribe: send mail to majordomo@FreeBSD.org
with "unsubscribe freebsd-current" in the body of the message




Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?Pine.SUN.3.91.1011110095953.20896A-100000>