Patch for Review: PR/9254 and other Win32 Threading Fixes
Boehm, Hans
hans_boehm@hp.com
Mon Jan 27 21:52:00 GMT 2003
Doesn't this reset the owner field to zero even for a nested _Jv_MutexUnlock call? I suspect you need a count field to correctly maintain the owner field, unless you can somehow get at the one that's already inside the CriticalSection.
Hans
> -----Original Message-----
> From: Ranjit Mathew [mailto:rmathew@hotmail.com]
> Sent: Monday, January 27, 2003 9:51 AM
> To: java-patches@gcc.gnu.org
> Cc: adam@megacz.com
> Subject: Re: Patch for Review: PR/9254 and other Win32 Threading Fixes
>
>
> Ranjit Mathew wrote:
> > This patch proposes to clean up the Win32 threads support in
> > libgcj a bit and contains fixes for a few problems reported
> > on the main list. These include:
>
>
> Resubmitting in "diff -u2" format on Adam's request:
>
> -------------------------------- 8< --------------------------------
> 2003-01-27 Ranjit Mathew <rmathew@hotmail.com>
>
> * include/win32-threads.h (_Jv_Mutex_t): Convert to a struct
> containing id of the owner thread as well.
> (_Jv_MutexInit): Initialise owner thread id to 0.
> (_Jv_MutexDestroy): Reset owner thread id to 0.
> (_Jv_MutexUnlock): Check if really the owner thread, reset
> owner thread id to 0 before leaving.
> (_Jv_MutexLock): Set owner thread id in the mutex.
> (_Jv_ThreadYield): Yield using Win32 Sleep(0).
>
> * win32-threads.cc (_Jv_CondWait): Check if really owner of
> the passed mutex.
> Pass handle of broadcast event, instead of a pointer to it
> in Win32 ResetEvent( ) call.
> Remove incorrect return values.
> (_Jv_CondDestroy): Close both event handles and delete
> critical section.
> (_Jv_CondNotify): Check if really the owner thread.
> (_Jv_CondNotifyAll): Check if really the owner thread.
> (_Jv_InitThreads): Change daemon_cond to a
> manual-reset event.
> (really_start): Use SetEvent( ) to signal daemon_cond.
> (_Jv_ThreadWait): Remove SignalObjectAndWait( ) and use
> WaitForSingleObject( ) instead to wait for daemon_cond to be
> signalled.
>
> --- include/win32-threads.h 2003-01-14 20:23:56.000000000 +0530
> +++ include/win32-threads.h 2003-01-27 20:57:05.000000000 +0530
> @@ -2,5 +2,6 @@
> // win32-threads.h - Defines for using Win32 threads.
>
> -/* Copyright (C) 1998, 1999, 2000 Free Software Foundation
> +/* Copyright (C) 1998, 1999, 2000, 2001, 2002, 2003 Free Software
> + Foundation
>
> This file is part of libgcj.
> @@ -19,11 +20,27 @@
> //
>
> -typedef struct _Jv_ConditionVariable_t {
> +typedef struct
> +{
> + // ev[0] (signal) is a Win32 auto-reset event for _Jv_CondNotify
> + // ev[1] (broadcast) is a Win32 manual-reset event for
> _Jv_CondNotifyAll
> HANDLE ev[2];
> - CRITICAL_SECTION count_mutex;
> +
> + // Number of threads waiting on this condition variable
> int blocked_count;
> -};
>
> -typedef CRITICAL_SECTION _Jv_Mutex_t;
> + // Protects access to the blocked_count variable
> + CRITICAL_SECTION count_mutex;
> +
> +} _Jv_ConditionVariable_t;
> +
> +typedef struct
> +{
> + // The thread-id of the owner thread if any, 0 otherwise
> + DWORD owner;
> +
> + // The actual Windows construct used to implement this mutex
> + CRITICAL_SECTION cs;
> +
> +} _Jv_Mutex_t;
>
> typedef struct
> @@ -61,5 +78,6 @@
> inline void _Jv_MutexInit (_Jv_Mutex_t *mu)
> {
> - InitializeCriticalSection(mu);
> + mu->owner = 0UL;
> + InitializeCriticalSection (&(mu->cs));
> }
>
> @@ -67,5 +85,6 @@
> inline void _Jv_MutexDestroy (_Jv_Mutex_t *mu)
> {
> - DeleteCriticalSection(mu);
> + mu->owner = 0UL;
> + DeleteCriticalSection (&(mu->cs));
> mu = NULL;
> }
> @@ -73,11 +92,18 @@
> inline int _Jv_MutexUnlock (_Jv_Mutex_t *mu)
> {
> - LeaveCriticalSection(mu);
> - return 0;
> + if (mu->owner == GetCurrentThreadId ( ))
> + {
> + mu->owner = 0UL;
> + LeaveCriticalSection (&(mu->cs));
> + return 0;
> + }
> + else
> + return 1;
> }
>
> inline int _Jv_MutexLock (_Jv_Mutex_t *mu)
> {
> - EnterCriticalSection(mu);
> + EnterCriticalSection (&(mu->cs));
> + mu->owner = GetCurrentThreadId ( );
> return 0;
> }
> @@ -105,7 +131,5 @@
> inline void _Jv_ThreadYield (void)
> {
> - // FIXME: win98 freezes hard (OS hang) when we use this --
> - // for now, we simply don't yield
> - // Sleep (0);
> + Sleep (0);
> }
>
> --- win32-threads.cc 2003-01-14 20:23:53.000000000 +0530
> +++ win32-threads.cc 2003-01-27 20:51:24.000000000 +0530
> @@ -1,5 +1,6 @@
> // win32-threads.cc - interface between libjava and Win32 threads.
>
> -/* Copyright (C) 1998, 1999 Free Software Foundation, Inc.
> +/* Copyright (C) 1998, 1999, 2000, 2001, 2002, 2003 Free Software
> + Foundation, Inc.
>
> This file is part of libgcj.
> @@ -71,10 +72,12 @@
> ensure_condvar_initialized(_Jv_ConditionVariable_t *cv)
> {
> - if (cv->ev[0] == 0) {
> - cv->ev[0] = CreateEvent (NULL, 0, 0, NULL);
> - if (cv->ev[0] == 0) JvFail("CreateEvent() failed");
> - cv->ev[1] = CreateEvent (NULL, 1, 0, NULL);
> - if (cv->ev[1] == 0) JvFail("CreateEvent() failed");
> - }
> + if (cv->ev[0] == 0)
> + {
> + cv->ev[0] = CreateEvent (NULL, 0, 0, NULL);
> + if (cv->ev[0] == 0) JvFail("CreateEvent() failed");
> +
> + cv->ev[1] = CreateEvent (NULL, 1, 0, NULL);
> + if (cv->ev[1] == 0) JvFail("CreateEvent() failed");
> + }
> }
>
> @@ -86,9 +89,11 @@
> _Jv_CondWait(_Jv_ConditionVariable_t *cv, _Jv_Mutex_t *mu,
> jlong millis, jint nanos)
> {
> + if (mu->owner != GetCurrentThreadId ( ))
> + return _JV_NOT_OWNER;
>
> - EnterCriticalSection(&cv->count_mutex);
> - ensure_condvar_initialized(cv);
> + EnterCriticalSection (&cv->count_mutex);
> + ensure_condvar_initialized (cv);
> cv->blocked_count++;
> - LeaveCriticalSection(&cv->count_mutex);
> + LeaveCriticalSection (&cv->count_mutex);
>
> DWORD time;
> @@ -103,16 +108,15 @@
> EnterCriticalSection(&cv->count_mutex);
> cv->blocked_count--;
> - // If we were unblocked by the second event (the broadcast
> one) and nobody is
> - // left, then reset the signal.
> - int last_waiter = rval == WAIT_OBJECT_0 + 1 &&
> cv->blocked_count == 0;
> + // If we were unblocked by the second event (the broadcast one)
> + // and nobody is left, then reset the event.
> + int last_waiter = (rval == (WAIT_OBJECT_0 + 1)) &&
> (cv->blocked_count == 0);
> LeaveCriticalSection(&cv->count_mutex);
>
> - if (last_waiter) ResetEvent(&cv->ev[1]);
> + if (last_waiter)
> + ResetEvent (cv->ev[1]);
>
> _Jv_MutexLock (mu);
>
> - if (rval == WAIT_FAILED) return GetLastError();
> - else if (rval == WAIT_TIMEOUT) return ETIMEDOUT;
> - else return 0;
> + return 0;
> }
>
> @@ -122,5 +126,5 @@
> // we do lazy creation of Events since CreateEvent() is
> insanely expensive
> cv->ev[0] = 0;
> - InitializeCriticalSection(&cv->count_mutex);
> + InitializeCriticalSection (&cv->count_mutex);
> cv->blocked_count = 0;
> }
> @@ -129,30 +133,49 @@
> _Jv_CondDestroy (_Jv_ConditionVariable_t *cv)
> {
> - if (cv->ev[0] != 0) CloseHandle(cv->ev[0]);
> + if (cv->ev[0] != 0)
> + {
> + CloseHandle (cv->ev[0]);
> + CloseHandle (cv->ev[1]);
> +
> + cv->ev[0] = 0;
> + }
> +
> + DeleteCriticalSection (&cv->count_mutex);
> +
> cv = NULL;
> }
>
> int
> -_Jv_CondNotify (_Jv_ConditionVariable_t *cv, _Jv_Mutex_t *)
> +_Jv_CondNotify (_Jv_ConditionVariable_t *cv, _Jv_Mutex_t *mu)
> {
> - EnterCriticalSection(&cv->count_mutex);
> - ensure_condvar_initialized(cv);
> + if (mu->owner != GetCurrentThreadId ( ))
> + return _JV_NOT_OWNER;
> +
> + EnterCriticalSection (&cv->count_mutex);
> + ensure_condvar_initialized (cv);
> int somebody_is_blocked = cv->blocked_count > 0;
> - LeaveCriticalSection(&cv->count_mutex);
> + LeaveCriticalSection (&cv->count_mutex);
> +
> + if (somebody_is_blocked)
> + SetEvent (cv->ev[0]);
>
> - if (somebody_is_blocked) return SetEvent (cv->ev[0]) ? 0 :
> GetLastError();
> - else return 0;
> + return 0;
> }
>
> int
> -_Jv_CondNotifyAll (_Jv_ConditionVariable_t *cv, _Jv_Mutex_t *)
> +_Jv_CondNotifyAll (_Jv_ConditionVariable_t *cv, _Jv_Mutex_t *mu)
> {
> - EnterCriticalSection(&cv->count_mutex);
> - ensure_condvar_initialized(cv);
> + if (mu->owner != GetCurrentThreadId ( ))
> + return _JV_NOT_OWNER;
> +
> + EnterCriticalSection (&cv->count_mutex);
> + ensure_condvar_initialized (cv);
> int somebody_is_blocked = cv->blocked_count > 0;
> - LeaveCriticalSection(&cv->count_mutex);
> + LeaveCriticalSection (&cv->count_mutex);
>
> - if (somebody_is_blocked) return SetEvent (cv->ev[1]) ? 0 :
> GetLastError();
> - else return 0;
> + if (somebody_is_blocked)
> + SetEvent (cv->ev[1]);
> +
> + return 0;
> }
>
> @@ -166,6 +189,6 @@
> _Jv_ThreadKey = TlsAlloc();
> _Jv_ThreadDataKey = TlsAlloc();
> - daemon_mutex = CreateMutex(NULL, 0, NULL);
> - daemon_cond = CreateEvent(NULL, 0, 0, NULL);
> + daemon_mutex = CreateMutex (NULL, 0, NULL);
> + daemon_cond = CreateEvent (NULL, 1, 0, NULL);
> non_daemon_count = 0;
> }
> @@ -256,5 +279,5 @@
> non_daemon_count--;
> if (! non_daemon_count)
> - PulseEvent (daemon_cond);
> + SetEvent (daemon_cond);
> ReleaseMutex (daemon_mutex);
> }
> @@ -298,8 +321,10 @@
> _Jv_ThreadWait (void)
> {
> - WaitForSingleObject(daemon_mutex, INFINITE);
> - if(non_daemon_count)
> - SignalObjectAndWait(daemon_mutex, daemon_cond, INFINITE, 0);
> - ReleaseMutex(daemon_mutex);
> + WaitForSingleObject (daemon_mutex, INFINITE);
> + if (non_daemon_count)
> + {
> + ReleaseMutex (daemon_mutex);
> + WaitForSingleObject (daemon_cond, INFINITE);
> + }
> }
>
> -------------------------------- 8< --------------------------------
>
> Sincerely Yours,
> Ranjit.
>
> --
> Ranjit Mathew Email: rmathew AT hotmail DOT com
> Bangalore,
> INDIA. Web: http://ranjitmathew.tripod.com/
>
>
More information about the Java-patches
mailing list