Skip site navigation (1)Skip section navigation (2)
Date:      Tue, 11 Dec 2001 05:02:54 -0800
From:      Bill Huey <billh@gnuppy.monkey.org>
To:        Bill Huey <billh@gnuppy.monkey.org>
Cc:        Nate Williams <nate@yogotech.com>, absinthe@pobox.com, shanon loveridge <shanon_loveridge@yahoo.co.uk>, freebsd-java@FreeBSD.ORG
Subject:   Re: Pthreads bug fix [was Re: jdk1.3.1p5]
Message-ID:  <20011211130254.GA9181@gnuppy>
In-Reply-To: <20011211115501.GA8729@gnuppy>
References:  <20011210001702.10731.qmail@web14303.mail.yahoo.com> <20011210024138.GA3148@gnuppy> <20011209223635.A1152@absinthe> <15380.15272.167683.46148@caddis.yogotech.com> <20011210003200.C1152@absinthe> <15380.65513.794203.276229@caddis.yogotech.com> <20011211115501.GA8729@gnuppy>

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

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

On Tue, Dec 11, 2001 at 03:55:01AM -0800, Bill Huey wrote:
> BTW, it's looking like I just fixed the pthreads bug with waiting-queue
> logic.

The CVS source changes to the JVM have been committed.

Here's a very sloppy preliminary patch for the pthreads stuff.

bill


--5vNYLRcllDrimb99
Content-Type: text/plain; charset=us-ascii
Content-Disposition: attachment; filename="pthread_full_fix.diff"

diff -urw 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	Mon Dec 10 04:33:46 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__;					\
+	enqueueStateEvent(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).
@@ -519,7 +536,8 @@
 	PS_SUSPENDED,
 	PS_DEAD,
 	PS_DEADLOCK,
-	PS_STATE_MAX
+	PS_STATE_MAX,
+	PS_REQUEST_WAITING_SUSPENDED
 };
 
 
@@ -653,6 +671,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 +898,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	remaining_wakeup_time;
+
+#define STATE_LOG_SIZE 32
+
+#ifdef EVENT_STATE_DEBUG
+	eventStateStructType state_log[STATE_LOG_SIZE];
+#endif
+
 };
 
+#ifdef EVENT_STATE_DEBUG
+static 
+void enqueueStateEvent(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 +1306,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 +1453,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 -urw 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	Tue Dec 11 03:32:50 2001
@@ -292,6 +292,7 @@
 			/* States which can timeout: */
 			case PS_COND_WAIT:
 			case PS_SLEEP_WAIT:
+			case PS_REQUEST_WAITING_SUSPENDED:
 				/* Restart the time slice: */
 				_thread_run->slice_usec = -1;
 
@@ -638,6 +639,10 @@
 	_thread_run->fname = fname;
 	_thread_run->lineno = lineno;
 
+#ifdef EVENT_STATE_DEBUG
+enqueueStateEvent(_thread_run, state, fname, lineno);
+#endif
+
 	/* Schedule the next thread that is ready: */
 	_thread_kern_sched(NULL);
 }
@@ -665,6 +670,10 @@
 	_thread_run->fname = fname;
 	_thread_run->lineno = lineno;
 
+#ifdef EVENT_STATE_DEBUG
+enqueueStateEvent(_thread_run, state, fname, lineno);
+#endif
+
 	_SPINUNLOCK(lock);
 
 	/* Schedule the next thread that is ready: */
@@ -997,14 +1006,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 *threadparam, const struct timespec *timeout)
+{
 	struct timespec current_time;
 	struct timeval  tv;
 
 	/* Reset the timeout flag for the running thread: */
-	_thread_run->timeout = 0;
+	threadparam->timeout = 0;
 
 	/* Check if the thread is to wait forever: */
 	if (timeout == NULL) {
@@ -1012,29 +1056,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;
+		threadparam->wakeup_time.tv_sec = -1;
+		threadparam->wakeup_time.tv_nsec = -1;
+
+		threadparam->remaining_wakeup_time.tv_sec  = -1;
+		threadparam->remaining_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;
+		threadparam->wakeup_time.tv_sec  = 0;
+		threadparam->wakeup_time.tv_nsec = 0;
+
+		threadparam->remaining_wakeup_time.tv_sec = 0;
+		threadparam->remaining_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(&threadparam->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
+		 */
+
+		threadparam->remaining_wakeup_time.tv_sec  = timeout->tv_sec;
+		threadparam->remaining_wakeup_time.tv_nsec = timeout->tv_nsec;
 	}
 }
 
Only in /usr/src/lib/libc_r/uthread: uthread_kern.c.orig
diff -urw 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	Tue Dec 11 03:36:10 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)
@@ -43,12 +45,21 @@
 	int	ret;
 	enum	pthread_susp old_suspended;
 
+fprintf(stderr, "pthread_resume_np\n");
 	/* Find the thread in the list of active threads: */
 	if ((ret = _find_thread(thread)) == 0) {
 		/* Cancel any pending suspensions: */
 		old_suspended = thread->suspended;
 		thread->suspended = SUSP_NO;
 
+		_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) {
 			/*
@@ -63,6 +74,24 @@
 				PTHREAD_SET_STATE(thread,PS_MUTEX_WAIT);
 				break;
 			case SUSP_COND_WAIT:
+		/* For cases where it was doing a pthread_cond_timedwait()
+		 * Mark the remaining suspend time.
+		 * --billh
+		 */
+#if 1
+		if ((thread->remaining_wakeup_time.tv_sec  == 0) 
+		 && (thread->remaining_wakeup_time.tv_nsec == 0)) {
+			_thread_kern_set_timeout_by_pthread_timespec(thread, &thread->remaining_wakeup_time);
+		}
+		else if (thread->remaining_wakeup_time.tv_sec == -1) {
+		}
+		else
+		{
+			_thread_kern_set_timeout_by_pthread_timespec(thread, &thread->remaining_wakeup_time);
+		}
+#endif
+			thread->suspended = SUSP_NO;
+
 				/* Set the thread's state back. */
 				PTHREAD_SET_STATE(thread,PS_COND_WAIT);
 				break;
@@ -91,6 +120,6 @@
 			_thread_kern_sig_undefer();
 		}
 	}
-	return(ret);
-}
+
 #endif
+
Only in /usr/src/lib/libc_r/uthread: uthread_resume_np.c.orig
diff -urw 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	Tue Dec 11 03:43:08 2001
@@ -38,12 +38,15 @@
 
 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)
 {
 	int ret;
 
+fprintf(stderr, "pthread_suspend_np\n");
 	/* Find the thread in the list of active threads: */
 	if ((ret = _find_thread(thread)) == 0) {
 		/*
@@ -52,6 +55,24 @@
 		 */
 		_thread_kern_sig_defer();
 
+		_pthread_suspend_np_by_pthread_common(thread);
+
+		/*
+		 * Undefer and handle pending signals, yielding if
+		 * necessary:
+		 */
+		_thread_kern_sig_undefer();
+	}
+fprintf(stderr, "pthread_suspend_np END\n");
+	return(ret);
+}
+
+
+void _pthread_suspend_np_by_pthread_common(pthread_t thread)
+{
+struct timeval  tv;
+struct timespec current_ts;
+
 		switch (thread->state) {
 		case PS_RUNNING:
 			/*
@@ -99,10 +120,42 @@
 			PTHREAD_SET_STATE(thread, PS_SUSPENDED);
 			break;
 		case PS_COND_WAIT:
+#if 1
+		/* This is for a pthreads_cond_timedwait() --billh */
+		if (thread->wakeup_time.tv_sec != -1) {
+			/* (1) Use to restore the waiting-queue time that's left when the
+			 * thread is resumed. --billh
+			 */
+			GET_CURRENT_TOD(tv);
+			TIMEVAL_TO_TIMESPEC(&tv, &current_ts);
+
+			_subtract_timespec3(&thread->remaining_wakeup_time, &thread->wakeup_time, &current_ts);
+
+			/* (2) So that it's inserted at the end of the waiting queue and
+			 * not scanned by the uthreads_kern.c waiting queue logic. It also
+			 * means to make it wait forever.
+			 */
+			thread->wakeup_time.tv_sec  = -1;
+			thread->wakeup_time.tv_nsec = -1;
+
+			/* (3) Remove and reinsert it at the end of waiting-queue
+			 * (automatic on the insertion attempt when (2)).
+			 */
+//			PTHREAD_WORKQ_REMOVE(thread);
+//			PTHREAD_WORKQ_INSERT(thread);
+		}
+		else
+		{
+			thread->remaining_wakeup_time.tv_sec = -1;
+			thread->remaining_wakeup_time.tv_nsec = -1;
+		}
+#endif
+
 			/* Mark the thread as suspended and still in a queue. */
 			thread->suspended = SUSP_COND_WAIT;
 
-			PTHREAD_SET_STATE(thread, PS_SUSPENDED);
+//			PTHREAD_SET_STATE(thread, PS_SUSPENDED);
+			PTHREAD_NEW_STATE(thread, PS_SUSPENDED);
 			break;
 		case PS_JOIN:
 			/* Mark the thread as suspended and joining: */
@@ -138,14 +191,6 @@
 			/* Nothing needs to be done: */
 			break;
 		}
-
-		/*
-		 * Undefer and handle pending signals, yielding if
-		 * necessary:
-		 */
-		_thread_kern_sig_undefer();
-	}
-	return(ret);
 }
 
 static void
Only in /usr/src/lib/libc_r/uthread: uthread_suspend_np.c.orig

--5vNYLRcllDrimb99--

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?20011211130254.GA9181>