* mutex.c (pthread_mutex_init): Remove critical

	section calls.
	(pthread_mutex_destroy): Likewise.
	(pthread_mutex_unlock): Likewise.
	(pthread_mutex_trylock): Likewise; recursive mutexes
	now increment the lock count rather than return EBUSY;
	errorcheck mutexes return EDEADLCK rather than
	EBUSY. This behaviour is consistent with the Solaris
	pthreads implementation.

	* implement.h (pthread_mutex_t_): Remove critical
	section element - no longer needed.

4 files changed, 61 insertions, 36 deletions

diff --git a/ANNOUNCE b/ANNOUNCE
index a6a84c5..6b9f0e5 100644
--- a/ANNOUNCE
+++ b/ANNOUNCE
@@ -31,7 +31,7 @@ platform dependence
 -------------------

 As Win9x doesn't provide one, the library now contains

 it's own InterlockedCompareExchange() routine. It is used to

-implement spinlocks and barriers, and soon may be used in mutexes.

+implement spinlocks and barriers, and also in mutexes.

 This routine relies on the CMPXCHG machine instruction which

 is only available in i486 or above CPUs. This library

 (from snapshot 20010712 onwards) therefore no longer runs on

@@ -53,6 +53,15 @@ Removed potential race condition in pthread_mutex_trylock and
 pthread_mutex_lock;

 - Alexander Terekhov <TEREKHOV@de.ibm.com>

 

+The behaviour of pthread_mutex_trylock in relation to

+recursive mutexes was inconsistent with commercial implementations.

+Trylock would return EBUSY if the lock was owned already

+regardless of mutex type. Trylock now increments the recursion

+count and returns 0 for RECURSIVE mutexes, and will now return

+EDEADLCK instead of EBUSY for ERRORCHECK mutexes. This is

+consistent with Solaris.

+- Thomas Pfaff <tpfaff@gmx.net>

+

 ---------------------------

 Known bugs in this snapshot

 ---------------------------

diff --git a/ChangeLog b/ChangeLog
index e9604a3..72593f2 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,22 @@
+2002-01-07  Ross Johnson  <rpj@setup1.ise.canberra.edu.au>
+
+	Contributed by - Thomas Pfaff <tpfaff@gmx.net>
+	                 Alexander Terekhov <TEREKHOV@de.ibm.com>
+
+	* mutex.c (pthread_mutex_init): Remove critical
+	section calls.
+	(pthread_mutex_destroy): Likewise.
+	(pthread_mutex_unlock): Likewise.
+	(pthread_mutex_trylock): Likewise; recursive mutexes
+	now increment the lock count rather than return EBUSY;
+	errorcheck mutexes return EDEADLCK rather than
+	EBUSY. This behaviour is consistent with the Solaris
+	pthreads implementation.
+
+	* implement.h (pthread_mutex_t_): Remove critical
+	section element - no longer needed.
+	
+
 2002-01-04  Ross Johnson  <rpj@setup1.ise.canberra.edu.au>
 
 	* attr.c (pthread_attr_setscope): Add more error
diff --git a/implement.h b/implement.h
index 3f2690e..e07306b 100644
--- a/implement.h
+++ b/implement.h
@@ -161,7 +161,6 @@ struct pthread_mutex_t_ {
   int kind;
   pthread_t ownerThread;
   HANDLE wait_sema;
-  CRITICAL_SECTION try_lock_cs;
 };
 
 struct pthread_mutexattr_t_ {
diff --git a/mutex.c b/mutex.c
index e62647e..7a232c5 100644
--- a/mutex.c
+++ b/mutex.c
@@ -135,12 +135,10 @@ pthread_mutex_init(pthread_mutex_t *mutex, const pthread_mutexattr_t *attr)
               ? PTHREAD_MUTEX_DEFAULT
               : (*attr)->kind);
   mx->ownerThread = NULL;
-  InitializeCriticalSection( &mx->try_lock_cs );
   mx->wait_sema = CreateSemaphore( NULL, 0, 1, NULL );
 
   if( NULL == mx->wait_sema )
     {
-      DeleteCriticalSection( &mx->try_lock_cs );
       result = EAGAIN;
     }
 
@@ -203,7 +201,6 @@ pthread_mutex_destroy(pthread_mutex_t *mutex)
 
           if (result == 0)
             {
-              DeleteCriticalSection( &mx->try_lock_cs );
               CloseHandle( mx->wait_sema );
               free(mx);
             }
@@ -702,16 +699,12 @@ pthread_mutex_unlock(pthread_mutex_t *mutex)
 	    {
 	      mx->ownerThread = NULL;
 	      
-              EnterCriticalSection( &mx->try_lock_cs );
-
-              if( InterlockedDecrement( &mx->lock_idx ) >= 0 )
-		{
-                  /* Someone is waiting on that mutex */
-                  ReleaseSemaphore( mx->wait_sema, 1, NULL );
-		}
-
-              LeaveCriticalSection( &mx->try_lock_cs );
-	    }
+          if( InterlockedDecrement( &mx->lock_idx ) >= 0 )
+          {
+             /* Someone is waiting on that mutex */
+             ReleaseSemaphore( mx->wait_sema, 1, NULL );
+          }
+        }
 	}
       else
 	{
@@ -752,30 +745,35 @@ pthread_mutex_trylock(pthread_mutex_t *mutex)
 
   if (result == 0)
     {
-      /* Try to lock only if mutex seems available */
-      if( PTW32_MUTEX_LOCK_IDX_INIT == mx->lock_idx )
-	{
-          EnterCriticalSection( &mx->try_lock_cs );
 
-          if( 0 == InterlockedIncrement( &mx->lock_idx ) )
-	    {
-              mx->recursive_count = 1;
-              mx->ownerThread = (mx->kind != PTHREAD_MUTEX_FAST_NP
-                                 ? pthread_self()
-                                 : (pthread_t) PTW32_MUTEX_OWNER_ANONYMOUS);
-	    }
-          else
-            {
-              (void) InterlockedDecrement( &mx->lock_idx );
-              result = EBUSY;
-            }
+      if( PTW32_MUTEX_LOCK_IDX_INIT == ptw32_InterlockedCompareExchange(
+             &mx->lock_idx, 0, PTW32_MUTEX_LOCK_IDX_INIT ) )
+      {
+         mx->recursive_count = 1;
+         mx->ownerThread = (mx->kind != PTHREAD_MUTEX_FAST_NP
+                            ? pthread_self()
+                            : (pthread_t) PTW32_MUTEX_OWNER_ANONYMOUS);
+      }
 
-          LeaveCriticalSection( &mx->try_lock_cs );
-        }
       else
-        {
-          result = EBUSY;
-        }
+      {
+         if( mx->kind != PTHREAD_MUTEX_FAST_NP &&
+             pthread_equal( mx->ownerThread, pthread_self() ) )
+         {
+            if( mx->kind == PTHREAD_MUTEX_RECURSIVE_NP )
+            {
+               mx->recursive_count++;
+            }
+            else
+            {
+               result = EDEADLK;
+            }
+         }
+         else
+         {
+            result = EBUSY;
+         }
+      }
     }
 
   return(result);