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, ¤t_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, ¤t_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, ¤t_ts); + + _subtract_timespec3(&thread->remaining_wakeup_time, &thread->wakeup_time, ¤t_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>