re PR java/9254 (java::lang::Object::wait(), threads-win32.cc returns wrong return codes)

2003-01-28  Ranjit Mathew  <rmathew@hotmail.com>

	Fixes PR java/9254:
	* include/win32-threads.h (_Jv_Mutex_t): Convert to a struct
	additionally containing id of the owner thread as well as
	the number of nested times the thread has acquired the mutex.
	(_Jv_MutexInit): Initialise owner thread id and refcount to 0.
	(_Jv_MutexDestroy): Reset owner thread id and refcount to 0.
	(_Jv_MutexUnlock): Check if really the owner thread, reset
	owner thread id to 0 before leaving, if leaving for the last
	time.
	(_Jv_MutexLock): Set owner thread id in the mutex and increment
	refcount.
	(_Jv_ThreadYield): Yield using a call to Sleep(0).
	* win32-threads.cc (_Jv_CondWait): Check if really owner of
	the passed mutex.
	Pass handle of the 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.

From-SVN: r62033

3 files changed, 136 insertions, 52 deletions

diff --git a/libjava/ChangeLog b/libjava/ChangeLog
index 8952b0b..dbcd959 100644
--- a/libjava/ChangeLog
+++ b/libjava/ChangeLog
@@ -1,3 +1,32 @@
+2003-01-28  Ranjit Mathew  <rmathew@hotmail.com>
+
+	Fixes PR java/9254:
+	* include/win32-threads.h (_Jv_Mutex_t): Convert to a struct
+	additionally containing id of the owner thread as well as
+	the number of nested times the thread has acquired the mutex.
+	(_Jv_MutexInit): Initialise owner thread id and refcount to 0.
+	(_Jv_MutexDestroy): Reset owner thread id and refcount to 0.
+	(_Jv_MutexUnlock): Check if really the owner thread, reset
+	owner thread id to 0 before leaving, if leaving for the last
+	time.
+	(_Jv_MutexLock): Set owner thread id in the mutex and increment
+	refcount.
+	(_Jv_ThreadYield): Yield using a call to Sleep(0).
+	* win32-threads.cc (_Jv_CondWait): Check if really owner of
+	the passed mutex.
+	Pass handle of the 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.
+
 2003-01-27  Ranjit Mathew  <rmathew@hotmail.com>
 
 	* configure.in: Specifically define HAVE_BACKTRACE if building
diff --git a/libjava/include/win32-threads.h b/libjava/include/win32-threads.h
index e8cf805..5e40ae2 100644
--- a/libjava/include/win32-threads.h
+++ b/libjava/include/win32-threads.h
@@ -1,7 +1,8 @@
 // -*- c++ -*-
 // 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.
 
@@ -18,13 +19,32 @@ details.  */
 // Typedefs.
 //
 
-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;
+
+  // Track nested mutex acquisitions by the same thread
+  int refcount;
+
+  // The actual Windows construct used to implement this mutex
+  CRITICAL_SECTION cs;
+
+} _Jv_Mutex_t;
 
 typedef struct
 {
@@ -60,25 +80,39 @@ int _Jv_CondNotifyAll (_Jv_ConditionVariable_t *cv, _Jv_Mutex_t *);
 
 inline void _Jv_MutexInit (_Jv_Mutex_t *mu)
 {
-  InitializeCriticalSection(mu);
+  mu->owner = 0UL;
+  mu->refcount = 0;
+  InitializeCriticalSection (&(mu->cs));
 }
 
 #define _Jv_HaveMutexDestroy
 inline void _Jv_MutexDestroy (_Jv_Mutex_t *mu)
 {
-  DeleteCriticalSection(mu);
+  mu->owner = 0UL;
+  mu->refcount = 0;
+  DeleteCriticalSection (&(mu->cs));
   mu = NULL;
 }
 
 inline int _Jv_MutexUnlock (_Jv_Mutex_t *mu)
 {
-  LeaveCriticalSection(mu);
-  return 0;
+  if (mu->owner == GetCurrentThreadId ( ))
+    {
+      mu->refcount--;
+      if (mu->refcount == 0)
+        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 ( );
+  mu->refcount++;
   return 0;
 }
 
@@ -104,9 +138,7 @@ inline _Jv_Thread_t *_Jv_ThreadCurrentData(void)
 
 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);
 }
 
 void _Jv_ThreadRegister (_Jv_Thread_t *data);
diff --git a/libjava/win32-threads.cc b/libjava/win32-threads.cc
index d31c191..3a3999a 100644
--- a/libjava/win32-threads.cc
+++ b/libjava/win32-threads.cc
@@ -1,6 +1,7 @@
 // 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.
 
@@ -70,12 +71,14 @@ DWORD _Jv_ThreadDataKey;
 inline void
 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");
+    }
 }
 
 // Reimplementation of the general algorithm described at
@@ -85,11 +88,13 @@ ensure_condvar_initialized(_Jv_ConditionVariable_t *cv)
 int
 _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;
   if ((millis == 0) && (nanos > 0)) time = 1;
@@ -102,18 +107,17 @@ _Jv_CondWait(_Jv_ConditionVariable_t *cv, _Jv_Mutex_t *mu, jlong millis, jint na
 
   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;
 }
 
 void
@@ -121,39 +125,56 @@ _Jv_CondInit (_Jv_ConditionVariable_t *cv)
 {
   // 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;
 }
 
 void
 _Jv_CondDestroy (_Jv_ConditionVariable_t *cv)
 {
-  if (cv->ev[0] != 0) CloseHandle(cv->ev[0]);
-  cv = NULL;
+  if (cv->ev[0] != 0)
+    {
+      CloseHandle (cv->ev[0]);
+      CloseHandle (cv->ev[1]);
+
+      cv->ev[0] = 0;
+    }
+
+  DeleteCriticalSection (&cv->count_mutex);
 }
 
 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;
 }
 
 //
@@ -165,8 +186,8 @@ _Jv_InitThreads (void)
 {
   _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;
 }
 
@@ -255,7 +276,7 @@ really_start (void* x)
       WaitForSingleObject (daemon_mutex, INFINITE);
       non_daemon_count--;
       if (! non_daemon_count)
-          PulseEvent (daemon_cond);
+        SetEvent (daemon_cond);
       ReleaseMutex (daemon_mutex);
     }
 
@@ -297,10 +318,12 @@ _Jv_ThreadStart (java::lang::Thread *thread, _Jv_Thread_t *data, _Jv_ThreadStart
 void
 _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);
+    }
 }
 
 void