diff options
-rw-r--r-- | libjava/ChangeLog | 10 | ||||
-rw-r--r-- | libjava/include/posix-threads.h | 7 | ||||
-rw-r--r-- | libjava/posix-threads.cc | 90 |
3 files changed, 66 insertions, 41 deletions
diff --git a/libjava/ChangeLog b/libjava/ChangeLog index 37153da..8d7ddc8 100644 --- a/libjava/ChangeLog +++ b/libjava/ChangeLog @@ -1,3 +1,13 @@ +2009-11-17 Andrew Haley <aph@redhat.com> + + * posix-threads.cc (park): Rewrite code to handle time. + Move mutex lock before the call to compare_and_swap to avoid a + race condition. + Add some assertions. + (unpark): Add an assertion. + (init): Move here from posix-threads.h. + * include/posix-threads.h (destroy): removed. + 2009-11-13 Eric Botcazou <ebotcazou@adacore.com> * exception.cc (PERSONALITY_FUNCTION): Fix oversight. diff --git a/libjava/include/posix-threads.h b/libjava/include/posix-threads.h index 806ee55..59e65f7 100644 --- a/libjava/include/posix-threads.h +++ b/libjava/include/posix-threads.h @@ -375,13 +375,6 @@ struct ParkHelper }; inline void -ParkHelper::init () -{ - pthread_mutex_init (&mutex, NULL); - pthread_cond_init (&cond, NULL); -} - -inline void ParkHelper::destroy () { pthread_mutex_destroy (&mutex); diff --git a/libjava/posix-threads.cc b/libjava/posix-threads.cc index 287d6b7..a26c3bd 100644 --- a/libjava/posix-threads.cc +++ b/libjava/posix-threads.cc @@ -359,15 +359,16 @@ ParkHelper::unpark () if (compare_and_swap (ptr, Thread::THREAD_PARK_RUNNING, Thread::THREAD_PARK_PERMIT)) return; - + /* If this thread is parked, put it into state RUNNING and send it a signal. */ - if (compare_and_swap + if (compare_and_swap (ptr, Thread::THREAD_PARK_PARKED, Thread::THREAD_PARK_RUNNING)) { pthread_mutex_lock (&mutex); - pthread_cond_signal (&cond); + int result = pthread_cond_signal (&cond); pthread_mutex_unlock (&mutex); + JvAssert (result == 0); } } @@ -380,6 +381,14 @@ ParkHelper::deactivate () permit = ::java::lang::Thread::THREAD_PARK_DEAD; } +void +ParkHelper::init () +{ + pthread_mutex_init (&mutex, NULL); + pthread_cond_init (&cond, NULL); + permit = ::java::lang::Thread::THREAD_PARK_RUNNING; +} + /** * Blocks the thread until a matching _Jv_ThreadUnpark() occurs, the * thread is interrupted or the optional timeout expires. If an @@ -407,32 +416,44 @@ ParkHelper::park (jboolean isAbsolute, jlong time) return; struct timespec ts; - jlong millis = 0, nanos = 0; if (time) { + unsigned long long seconds; + unsigned long usec; + if (isAbsolute) { - millis = time; - nanos = 0; + ts.tv_sec = time / 1000; + ts.tv_nsec = (time % 1000) * 1000 * 1000; } else { - millis = java::lang::System::currentTimeMillis(); - nanos = time; - } - - if (millis > 0 || nanos > 0) - { // Calculate the abstime corresponding to the timeout. - // Everything is in milliseconds. - // - // We use `unsigned long long' rather than jlong because our - // caller may pass up to Long.MAX_VALUE millis. This would - // overflow the range of a timespec. + jlong nanos = time; + jlong millis = 0; - unsigned long long m = (unsigned long long)millis; - unsigned long long seconds = m / 1000; + // For better accuracy, should use pthread_condattr_setclock + // and clock_gettime. +#ifdef HAVE_GETTIMEOFDAY + timeval tv; + gettimeofday (&tv, NULL); + usec = tv.tv_usec; + seconds = tv.tv_sec; +#else + unsigned long long startTime + = java::lang::System::currentTimeMillis(); + seconds = startTime / 1000; + /* Assume we're about half-way through this millisecond. */ + usec = (startTime % 1000) * 1000 + 500; +#endif + /* These next two statements cannot overflow. */ + usec += nanos / 1000; + usec += (millis % 1000) * 1000; + /* These two statements could overflow only if tv.tv_sec was + insanely large. */ + seconds += millis / 1000; + seconds += usec / 1000000; ts.tv_sec = seconds; if (ts.tv_sec < 0 || (unsigned long long)ts.tv_sec != seconds) @@ -442,29 +463,30 @@ ParkHelper::park (jboolean isAbsolute, jlong time) millis = nanos = 0; } else - { - m %= 1000; - ts.tv_nsec = m * 1000000 + (unsigned long long)nanos; - } + /* This next statement also cannot overflow. */ + ts.tv_nsec = (usec % 1000000) * 1000 + (nanos % 1000); } } - + + pthread_mutex_lock (&mutex); if (compare_and_swap (ptr, Thread::THREAD_PARK_RUNNING, Thread::THREAD_PARK_PARKED)) { - pthread_mutex_lock (&mutex); - if (millis == 0 && nanos == 0) - pthread_cond_wait (&cond, &mutex); + int result = 0; + + if (! time) + result = pthread_cond_wait (&cond, &mutex); else - pthread_cond_timedwait (&cond, &mutex, &ts); - pthread_mutex_unlock (&mutex); - + result = pthread_cond_timedwait (&cond, &mutex, &ts); + + JvAssert (result == 0 || result == ETIMEDOUT); + /* If we were unparked by some other thread, this will already - be in state THREAD_PARK_RUNNING. If we timed out, we have to - do it ourself. */ - compare_and_swap - (ptr, Thread::THREAD_PARK_PARKED, Thread::THREAD_PARK_RUNNING); + be in state THREAD_PARK_RUNNING. If we timed out or were + interrupted, we have to do it ourself. */ + permit = Thread::THREAD_PARK_RUNNING; } + pthread_mutex_unlock (&mutex); } static void |