Skip site navigation (1)Skip section navigation (2)
Date:      Thu, 13 Dec 2001 02:53:26 -0800
From:      Bill Huey <billh@gnuppy.monkey.org>
To:        Daniel Eischen <eischen@pcnet1.pcnet.com>
Cc:        Bill Huey <billh@gnuppy.monkey.org>, freebsd-java@FreeBSD.ORG
Subject:   pthreads bug fix revised
Message-ID:  <20011213105326.GA15665@gnuppy>
In-Reply-To: <Pine.SUN.3.91.1011212071002.11047A-100000@pcnet1.pcnet.com>
References:  <20011212074342.GB4677@gnuppy> <Pine.SUN.3.91.1011212071002.11047A-100000@pcnet1.pcnet.com>

next in thread | previous in thread | raw e-mail | index | archive | help

--IJpNTDwzlM2Ie8A6
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline


Hey,

This is the same pthreads patch, but cleaned up a bit for developer
use only. It has been simplified to deal with absolute time out values
correctly in the Posix API, since I misinterpreted the solution
to the problem the first place.

Hopefully, a better or more correct patch will replace this soon
from Dan.

bill


--IJpNTDwzlM2Ie8A6
Content-Type: text/plain; charset=us-ascii
Content-Disposition: attachment; filename="pthread_fix_absolute_timespec.diff"

diff -ru libc_r.clean/uthread/pthread_private.h /usr/src/lib/libc_r/uthread/pthread_private.h
--- libc_r.clean/uthread/pthread_private.h	Sat Dec  8 19:12:35 2001
+++ /usr/src/lib/libc_r/uthread/pthread_private.h	Thu Dec 13 02:20:33 2001
@@ -60,6 +60,10 @@
 #include <spinlock.h>
 #include <pthread_np.h>
 
+#define EVENT_STATE_DEBUG
+
+#include <string.h>
+
 /*
  * Define machine dependent macros to get and set the stack pointer
  * from the supported contexts.  Also define a macro to set the return
@@ -162,12 +166,25 @@
 /*
  * State change macro without scheduling queue change:
  */
+#ifdef EVENT_STATE_DEBUG
+
+#define PTHREAD_SET_STATE(thrd, newstate) do {				\
+	(thrd)->state = newstate;					\
+	(thrd)->fname = __FILE__;					\
+	(thrd)->lineno = __LINE__;					\
+	_debug_enqueue_state_event(thrd, newstate,__FILE__, __LINE__ );		\
+} while (0)
+
+#else
+
 #define PTHREAD_SET_STATE(thrd, newstate) do {				\
 	(thrd)->state = newstate;					\
 	(thrd)->fname = __FILE__;					\
 	(thrd)->lineno = __LINE__;					\
 } while (0)
 
+#endif
+
 /*
  * State change macro with scheduling queue change - This must be
  * called with preemption deferred (see thread_kern_sched_[un]defer).
@@ -653,6 +670,15 @@
 	siginfo_t		siginfo;
 };
 
+#ifdef EVENT_STATE_DEBUG
+typedef struct eventStateStruct
+{
+	enum pthread_state	state;
+	char			*fname;
+	int			lineno;
+} eventStateStructType;
+#endif
+
 /*
  * Thread structure.
  */
@@ -871,8 +897,34 @@
 	struct pthread_cleanup *cleanup;
 	char			*fname;	/* Ptr to source file name  */
 	int			lineno;	/* Source line number.      */
+
+	/*
+	 * Record the remaining at suspend points so that waits can be
+	 * properly resumed. --billh
+	 */
+	struct timespec	previous_wakeup_time;
+
+#define STATE_LOG_SIZE 32
+
+#ifdef EVENT_STATE_DEBUG
+	eventStateStructType state_log[STATE_LOG_SIZE];
+#endif
+
 };
 
+#ifdef EVENT_STATE_DEBUG
+static 
+void _debug_enqueue_state_event(pthread_t thread, enum pthread_state state, char *fname, int lineno)
+{
+	memmove(&thread->state_log[1],
+		&thread->state_log[0],
+		sizeof(eventStateStructType)*(STATE_LOG_SIZE -1));
+	thread->state_log[0].state = state;
+	thread->state_log[0].fname = fname;
+	thread->state_log[0].lineno = lineno;
+}
+#endif
+
 /* Spare thread stack. */
 struct stack {
 	SLIST_ENTRY(stack)	qe; /* Queue entry for this stack. */
@@ -1253,7 +1305,9 @@
 void    _thread_kern_sched_state(enum pthread_state, char *fname, int lineno);
 void	_thread_kern_sched_state_unlock(enum pthread_state state,
 	    spinlock_t *lock, char *fname, int lineno);
+void	_wrap_timespec(const struct timespec *tspec);
 void    _thread_kern_set_timeout(const struct timespec *);
+void	_thread_kern_set_timeout_by_pthread_timespec(struct pthread *, const struct timespec *);
 void    _thread_kern_sig_defer(void);
 void    _thread_kern_sig_undefer(void);
 void    _thread_sig_handler(int, siginfo_t *, ucontext_t *);
@@ -1398,4 +1452,9 @@
 #endif
 __END_DECLS
 
+/* timespec math additions --billh */
+void _add_timespec3     (struct timespec *, const struct timespec *, const struct timespec *);
+void _subtract_timespec3(struct timespec *, const struct timespec *, const struct timespec *);
+
 #endif  /* !_PTHREAD_PRIVATE_H */
+
Only in /usr/src/lib/libc_r/uthread: pthread_private.h.orig
diff -ru libc_r.clean/uthread/uthread_kern.c /usr/src/lib/libc_r/uthread/uthread_kern.c
--- libc_r.clean/uthread/uthread_kern.c	Sat Dec  8 19:12:35 2001
+++ /usr/src/lib/libc_r/uthread/uthread_kern.c	Thu Dec 13 02:12:22 2001
@@ -386,6 +386,12 @@
 				/* Return zero file descriptors ready: */
 				pthread->data.poll_data->nfds = 0;
 				/* fall through */
+#ifdef 0
+			case PS_SUSPENDED:
+				PTHREAD_WAITQ_REMOVE(pthread);
+				pthread->suspended = SUSP_YES;
+				break;
+#endif
 			default:
 				/*
 				 * Remove this thread from the waiting queue
@@ -638,6 +644,10 @@
 	_thread_run->fname = fname;
 	_thread_run->lineno = lineno;
 
+#ifdef EVENT_STATE_DEBUG
+_debug_enqueue_state_event(_thread_run, state, fname, lineno);
+#endif
+
 	/* Schedule the next thread that is ready: */
 	_thread_kern_sched(NULL);
 }
@@ -665,6 +675,10 @@
 	_thread_run->fname = fname;
 	_thread_run->lineno = lineno;
 
+#ifdef EVENT_STATE_DEBUG
+_debug_enqueue_state_event(_thread_run, state, fname, lineno);
+#endif
+
 	_SPINUNLOCK(lock);
 
 	/* Schedule the next thread that is ready: */
@@ -997,14 +1011,49 @@
 	}
 }
 
+
+#define NSEC_UPPERLIMIT 1000000000
+
+void _add_timespec3(struct timespec *a, const struct timespec *b, const struct timespec *c)
+{
+	a->tv_nsec = b->tv_nsec + c->tv_nsec;
+	a->tv_sec  = b->tv_sec  + c->tv_sec;
+
+	/* carry the result into the "tv_sec" --billh */
+	if (a->tv_nsec >= NSEC_UPPERLIMIT) {
+		a->tv_sec  += 1;
+		a->tv_nsec -= NSEC_UPPERLIMIT;
+	}
+
+}
+
+void _subtract_timespec3(struct timespec *a, const struct timespec *b, const struct timespec *c)
+{
+	a->tv_nsec = b->tv_nsec - c->tv_nsec;
+	a->tv_sec  = b->tv_sec  - c->tv_sec;
+
+	/* borrow from "tv_sec". --billh */
+	if (a->tv_nsec < 0) {
+		a->tv_sec  -= 1;
+		a->tv_nsec += NSEC_UPPERLIMIT;
+	}
+
+}
+
 void
 _thread_kern_set_timeout(const struct timespec * timeout)
 {
+	_thread_kern_set_timeout_by_pthread_timespec(_thread_run, timeout);
+}
+
+void
+_thread_kern_set_timeout_by_pthread_timespec(struct pthread *thread, const struct timespec *timeout)
+{
 	struct timespec current_time;
 	struct timeval  tv;
 
 	/* Reset the timeout flag for the running thread: */
-	_thread_run->timeout = 0;
+	thread->timeout = 0;
 
 	/* Check if the thread is to wait forever: */
 	if (timeout == NULL) {
@@ -1012,29 +1061,35 @@
 		 * Set the wakeup time to something that can be recognised as
 		 * different to an actual time of day:
 		 */
-		_thread_run->wakeup_time.tv_sec = -1;
-		_thread_run->wakeup_time.tv_nsec = -1;
+		thread->wakeup_time.tv_sec = -1;
+		thread->wakeup_time.tv_nsec = -1;
+
+		thread->previous_wakeup_time.tv_sec  = -1;
+		thread->previous_wakeup_time.tv_nsec = -1;
 	}
 	/* Check if no waiting is required: */
 	else if (timeout->tv_sec == 0 && timeout->tv_nsec == 0) {
 		/* Set the wake up time to 'immediately': */
-		_thread_run->wakeup_time.tv_sec = 0;
-		_thread_run->wakeup_time.tv_nsec = 0;
+		thread->wakeup_time.tv_sec  = 0;
+		thread->wakeup_time.tv_nsec = 0;
+
+		thread->previous_wakeup_time.tv_sec = 0;
+		thread->previous_wakeup_time.tv_nsec = 0;
 	} else {
 		/* Get the current time: */
 		GET_CURRENT_TOD(tv);
 		TIMEVAL_TO_TIMESPEC(&tv, &current_time);
 
 		/* Calculate the time for the current thread to wake up: */
-		_thread_run->wakeup_time.tv_sec = current_time.tv_sec + timeout->tv_sec;
-		_thread_run->wakeup_time.tv_nsec = current_time.tv_nsec + timeout->tv_nsec;
+		_add_timespec3(&thread->wakeup_time, &current_time, timeout);
 
-		/* Check if the nanosecond field needs to wrap: */
-		if (_thread_run->wakeup_time.tv_nsec >= 1000000000) {
-			/* Wrap the nanosecond field: */
-			_thread_run->wakeup_time.tv_sec += 1;
-			_thread_run->wakeup_time.tv_nsec -= 1000000000;
-		}
+		/* Set the timeout length of this wake up operation so that it can be
+		 * properly recalculated after being suspended and put at the tail of the
+		 * waiting queue. --billh
+		 */
+
+		thread->previous_wakeup_time.tv_sec  = timeout->tv_sec;
+		thread->previous_wakeup_time.tv_nsec = timeout->tv_nsec;
 	}
 }
 
Only in /usr/src/lib/libc_r/uthread: uthread_kern.c.orig
diff -ru libc_r.clean/uthread/uthread_resume_np.c /usr/src/lib/libc_r/uthread/uthread_resume_np.c
--- libc_r.clean/uthread/uthread_resume_np.c	Sat Dec  8 19:12:35 2001
+++ /usr/src/lib/libc_r/uthread/uthread_resume_np.c	Thu Dec 13 02:10:24 2001
@@ -36,6 +36,8 @@
 #include <pthread.h>
 #include "pthread_private.h"
 
+void _pthread_resume_by_pthread_common(pthread_t, enum pthread_susp );
+
 /* Resume a thread: */
 int
 pthread_resume_np(pthread_t thread)
@@ -49,48 +51,67 @@
 		old_suspended = thread->suspended;
 		thread->suspended = SUSP_NO;
 
-		/* Is it currently suspended? */
-		if (thread->state == PS_SUSPENDED) {
-			/*
-			 * Defer signals to protect the scheduling queues
-			 * from access by the signal handler:
-			 */
-			_thread_kern_sig_defer();
-
-			switch (old_suspended) {
-			case SUSP_MUTEX_WAIT:
-				/* Set the thread's state back. */
-				PTHREAD_SET_STATE(thread,PS_MUTEX_WAIT);
-				break;
-			case SUSP_COND_WAIT:
-				/* Set the thread's state back. */
-				PTHREAD_SET_STATE(thread,PS_COND_WAIT);
-				break;
-			case SUSP_JOIN:
-				/* Set the thread's state back. */
-				PTHREAD_SET_STATE(thread,PS_JOIN);
-				break;
-			case SUSP_NOWAIT:
-				/* Allow the thread to run. */
-				PTHREAD_SET_STATE(thread,PS_RUNNING);
-				PTHREAD_WAITQ_REMOVE(thread);
-				PTHREAD_PRIOQ_INSERT_TAIL(thread);
-				break;
-			case SUSP_NO:
-			case SUSP_YES:
-				/* Allow the thread to run. */
-				PTHREAD_SET_STATE(thread,PS_RUNNING);
-				PTHREAD_PRIOQ_INSERT_TAIL(thread);
-				break;
-			}
-
-			/*
-			 * Undefer and handle pending signals, yielding if
-			 * necessary:
-			 */
-			_thread_kern_sig_undefer();
-		}
+		_pthread_resume_by_pthread_common(thread, old_suspended);
 	}
+fprintf(stderr, "pthread_resume_np END\n");
 	return(ret);
 }
+
+void _pthread_resume_by_pthread_common(pthread_t thread, enum pthread_susp old_suspended)
+{
+	/* Is it currently suspended? */
+	if (thread->state == PS_SUSPENDED) {
+	/*
+	 * Defer signals to protect the scheduling queues
+	 * from access by the signal handler:
+	 */
+	_thread_kern_sig_defer();
+
+	switch (old_suspended) {
+		case SUSP_MUTEX_WAIT:
+			/* Set the thread's state back. */
+			PTHREAD_SET_STATE(thread,PS_MUTEX_WAIT);
+			break;
+		case SUSP_COND_WAIT:
+		/* For cases where it was doing a pthread_cond_timedwait()
+		 * restor the previous suspend time.
+		 * --billh
+		 */
+			thread->wakeup_time.tv_sec  = thread->previous_wakeup_time.tv_sec;
+			thread->wakeup_time.tv_nsec = thread->previous_wakeup_time.tv_nsec;
+			thread->previous_wakeup_time.tv_sec  = -1;
+			thread->previous_wakeup_time.tv_nsec = -1;
+
+			thread->suspended = SUSP_NO;
+
+			/* Set the thread's state back. */
+			PTHREAD_SET_STATE(thread,PS_COND_WAIT);
+			break;
+		case SUSP_JOIN:
+			/* Set the thread's state back. */
+			PTHREAD_SET_STATE(thread,PS_JOIN);
+			break;
+		case SUSP_NOWAIT:
+			/* Allow the thread to run. */
+			PTHREAD_SET_STATE(thread,PS_RUNNING);
+			PTHREAD_WAITQ_REMOVE(thread);
+			PTHREAD_PRIOQ_INSERT_TAIL(thread);
+			break;
+		case SUSP_NO:
+		case SUSP_YES:
+			/* Allow the thread to run. */
+			PTHREAD_SET_STATE(thread,PS_RUNNING);
+			PTHREAD_PRIOQ_INSERT_TAIL(thread);
+			break;
+		}
+
+	/*
+	 * Undefer and handle pending signals, yielding if
+	 * necessary:
+	 */
+	_thread_kern_sig_undefer();
+	}
+}
+
 #endif
+
Only in /usr/src/lib/libc_r/uthread: uthread_resume_np.c.orig
diff -ru libc_r.clean/uthread/uthread_suspend_np.c /usr/src/lib/libc_r/uthread/uthread_suspend_np.c
--- libc_r.clean/uthread/uthread_suspend_np.c	Sat Dec  8 19:12:35 2001
+++ /usr/src/lib/libc_r/uthread/uthread_suspend_np.c	Thu Dec 13 02:10:04 2001
@@ -38,6 +38,8 @@
 
 static void	finish_suspension(void *arg);
 
+void _pthread_suspend_np_by_pthread_common(pthread_t thread);
+
 /* Suspend a thread: */
 int
 pthread_suspend_np(pthread_t thread)
@@ -52,92 +54,7 @@
 		 */
 		_thread_kern_sig_defer();
 
-		switch (thread->state) {
-		case PS_RUNNING:
-			/*
-			 * Remove the thread from the priority queue and
-			 * set the state to suspended:
-			 */
-			PTHREAD_PRIOQ_REMOVE(thread);
-			PTHREAD_SET_STATE(thread, PS_SUSPENDED);
-			break;
-
-		case PS_SPINBLOCK:
-		case PS_FDR_WAIT:
-		case PS_FDW_WAIT:
-		case PS_POLL_WAIT:
-		case PS_SELECT_WAIT:
-			/*
-			 * Remove these threads from the work queue
-			 * and mark the operation as interrupted:
-			 */
-			if ((thread->flags & PTHREAD_FLAGS_IN_WORKQ) != 0)
-				PTHREAD_WORKQ_REMOVE(thread);
-			_thread_seterrno(thread,EINTR);
-
-			/* FALLTHROUGH */
-		case PS_SLEEP_WAIT:
-			thread->interrupted = 1;
-
-			/* FALLTHROUGH */
-		case PS_SIGTHREAD:
-		case PS_WAIT_WAIT:
-		case PS_SIGSUSPEND:
-		case PS_SIGWAIT:
-			/*
-			 * Remove these threads from the waiting queue and
-			 * set their state to suspended:
-			 */
-			PTHREAD_WAITQ_REMOVE(thread);
-			PTHREAD_SET_STATE(thread, PS_SUSPENDED);
-			break;
-
-		case PS_MUTEX_WAIT:
-			/* Mark the thread as suspended and still in a queue. */
-			thread->suspended = SUSP_MUTEX_WAIT;
-
-			PTHREAD_SET_STATE(thread, PS_SUSPENDED);
-			break;
-		case PS_COND_WAIT:
-			/* Mark the thread as suspended and still in a queue. */
-			thread->suspended = SUSP_COND_WAIT;
-
-			PTHREAD_SET_STATE(thread, PS_SUSPENDED);
-			break;
-		case PS_JOIN:
-			/* Mark the thread as suspended and joining: */
-			thread->suspended = SUSP_JOIN;
-
-			PTHREAD_NEW_STATE(thread, PS_SUSPENDED);
-			break;
-		case PS_FDLR_WAIT:
-		case PS_FDLW_WAIT:
-		case PS_FILE_WAIT:
-			/* Mark the thread as suspended: */
-			thread->suspended = SUSP_YES;
-
-			/*
-			 * Threads in these states may be in queues.
-			 * In order to preserve queue integrity, the
-			 * cancelled thread must remove itself from the
-			 * queue.  Mark the thread as interrupted and
-			 * set the state to running.  When the thread
-			 * resumes, it will remove itself from the queue
-			 * and call the suspension completion routine.
-			 */
-			thread->interrupted = 1;
-			_thread_seterrno(thread, EINTR);
-			PTHREAD_NEW_STATE(thread, PS_RUNNING);
-			thread->continuation = finish_suspension;
-			break;
-
-		case PS_DEAD:
-		case PS_DEADLOCK:
-		case PS_STATE_MAX:
-		case PS_SUSPENDED:
-			/* Nothing needs to be done: */
-			break;
-		}
+		_pthread_suspend_np_by_pthread_common(thread);
 
 		/*
 		 * Undefer and handle pending signals, yielding if
@@ -145,7 +62,104 @@
 		 */
 		_thread_kern_sig_undefer();
 	}
+fprintf(stderr, "pthread_suspend_np END\n");
 	return(ret);
+}
+
+
+void _pthread_suspend_np_by_pthread_common(pthread_t thread)
+{
+	switch (thread->state) {
+	case PS_RUNNING:
+		/*
+		 * Remove the thread from the priority queue and
+		 * set the state to suspended:
+		 */
+		PTHREAD_PRIOQ_REMOVE(thread);
+		PTHREAD_SET_STATE(thread, PS_SUSPENDED);
+		break;
+
+	case PS_SPINBLOCK:
+	case PS_FDR_WAIT:
+	case PS_FDW_WAIT:
+	case PS_POLL_WAIT:
+	case PS_SELECT_WAIT:
+		/*
+		 * Remove these threads from the work queue
+		 * and mark the operation as interrupted:
+		 */
+		if ((thread->flags & PTHREAD_FLAGS_IN_WORKQ) != 0)
+			PTHREAD_WORKQ_REMOVE(thread);
+		_thread_seterrno(thread,EINTR);
+
+		/* FALLTHROUGH */
+	case PS_SLEEP_WAIT:
+		thread->interrupted = 1;
+
+		/* FALLTHROUGH */
+	case PS_SIGTHREAD:
+	case PS_WAIT_WAIT:
+	case PS_SIGSUSPEND:
+	case PS_SIGWAIT:
+		/*
+		 * Remove these threads from the waiting queue and
+		 * set their state to suspended:
+		 */
+		PTHREAD_WAITQ_REMOVE(thread);
+		PTHREAD_SET_STATE(thread, PS_SUSPENDED);
+		break;
+
+	case PS_MUTEX_WAIT:
+		/* Mark the thread as suspended and still in a queue. */
+		thread->suspended = SUSP_MUTEX_WAIT;
+
+		PTHREAD_SET_STATE(thread, PS_SUSPENDED);
+		break;
+	case PS_COND_WAIT:
+		thread->previous_wakeup_time.tv_nsec = thread->wakeup_time.tv_nsec;
+		thread->previous_wakeup_time.tv_sec  = thread->wakeup_time.tv_sec;
+
+		thread->wakeup_time.tv_nsec = -1;
+		thread->wakeup_time.tv_sec  = -1;
+
+		/* Mark the thread as suspended and still in a queue. */
+		thread->suspended = SUSP_COND_WAIT;
+		PTHREAD_NEW_STATE(thread, PS_SUSPENDED);
+		break;
+	case PS_JOIN:
+		/* Mark the thread as suspended and joining: */
+		thread->suspended = SUSP_JOIN;
+
+		PTHREAD_NEW_STATE(thread, PS_SUSPENDED);
+		break;
+	case PS_FDLR_WAIT:
+	case PS_FDLW_WAIT:
+	case PS_FILE_WAIT:
+		/* Mark the thread as suspended: */
+		thread->suspended = SUSP_YES;
+
+		/*
+		 * Threads in these states may be in queues.
+		 * In order to preserve queue integrity, the
+		 * cancelled thread must remove itself from the
+		 * queue.  Mark the thread as interrupted and
+		 * set the state to running.  When the thread
+		 * resumes, it will remove itself from the queue
+		 * and call the suspension completion routine.
+		 */
+		thread->interrupted = 1;
+		_thread_seterrno(thread, EINTR);
+		PTHREAD_NEW_STATE(thread, PS_RUNNING);
+		thread->continuation = finish_suspension;
+		break;
+
+	case PS_DEAD:
+	case PS_DEADLOCK:
+	case PS_STATE_MAX:
+	case PS_SUSPENDED:
+		/* Nothing needs to be done: */
+		break;
+	}
 }
 
 static void
Only in /usr/src/lib/libc_r/uthread: uthread_suspend_np.c.orig

--IJpNTDwzlM2Ie8A6--

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




Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20011213105326.GA15665>