This patch contains a new mutex implementation.

The advantages are:

- Same code on Win9x and NT. Actual are critical sections used on NT and
  kernel mutexes on 9x.
- Posix compliant error codes.
- State is preserved after fork as it should.
- Supports both errorchecking and recursive mutexes.
- Should be at least as fast as critical sections.

Unfortunately the pthread_mutex_trylock call requires
InterlockedCompareExchange that is not available on Win95.
See my winbase patch for a workaround.

Just like critical sections it will use a counter and a semaphore to block
other threads. The semaphore is only used when at least one thread is
waiting, otherwise a kernel transition is not needed.

With these mutexes the default type has changed from recursive to
errorchecking, this is also the default on all other pthread platforms
that i know (except Linux where the default is FAST where you will not get
an EDEADLOCK on locking twice, it WILL deadlock instead).

With my previous 2 patches and this one i was able to build and run a
threaded perl that has passed all tests (to be true it failed 3 but these
were really not pthread related).

2002-08-15  Thomas Pfaff <[EMAIL PROTECTED]>

        * include/pthread.h: Added define for errorchecking mutexes,
        changed default mutex type.
        * thread.cc (pthread_mutex::pthread_mutex): New implemented.
        (pthread_mutex::~pthread_mutex): Ditto.
        (pthread_mutex::Lock): Ditto.
        (pthread_mutex::TryLock): Ditto.
        (pthread_mutex::UnLock): Ditto.
        (pthread_mutex::Destroy): New method.
        (pthread_mutex::SetOwner): Ditto.
        (pthread_mutex::fixup_after_fork): Preserve state after fork.
        (__pthread_mutex_destroy): Call pthread_mutex::Destroy to destroy
        mutex.
        (__pthread_mutexattr_settype): Allow errorchecking and recursive
        types.
        * thread.h (pthread_mutex::criticalsection): Removed.
        (pthread_mutex::lock_counter): New member.
        (pthread_mutex::recursion_counter): Ditto.
        (pthread_mutex::owner): Ditto.
        (pthread_mutex::type): Ditto.
        (pthread_mutex::Destroy): New method.
        (pthread_mutex::SetOwner): Ditto.

diff -urp src.old/winsup/cygwin/include/pthread.h src/winsup/cygwin/include/pthread.h
--- src.old/winsup/cygwin/include/pthread.h     Wed Aug 14 14:19:59 2002
+++ src/winsup/cygwin/include/pthread.h Wed Aug 14 14:32:08 2002
@@ -50,12 +50,11 @@ extern "C"
 #define PTHREAD_CREATE_JOINABLE 0
 #define PTHREAD_EXPLICIT_SCHED 1
 #define PTHREAD_INHERIT_SCHED 0
-#define PTHREAD_MUTEX_DEFAULT 0
-#define PTHREAD_MUTEX_ERRORCHECK 1
-#define PTHREAD_MUTEX_NORMAL 2
+#define PTHREAD_MUTEX_ERRORCHECK 0
+#define PTHREAD_MUTEX_RECURSIVE 1
+#define PTHREAD_MUTEX_DEFAULT PTHREAD_MUTEX_ERRORCHECK
 /* this should be too low to ever be a valid address */
 #define PTHREAD_MUTEX_INITIALIZER (void *)20
-#define PTHREAD_MUTEX_RECURSIVE 0
 #define PTHREAD_ONCE_INIT { PTHREAD_MUTEX_INITIALIZER, 0 }
 #define PTHREAD_PRIO_INHERIT
 #define PTHREAD_PRIO_NONE
diff -urp src.old/winsup/cygwin/thread.cc src/winsup/cygwin/thread.cc
--- src.old/winsup/cygwin/thread.cc     Wed Aug 14 14:31:13 2002
+++ src/winsup/cygwin/thread.cc Wed Aug 14 14:32:08 2002
@@ -1042,39 +1042,40 @@ pthread_key::fixup_after_fork ()
  *Isn't duplicated, it's reopened.
  */
 
-pthread_mutex::pthread_mutex (pthread_mutexattr *attr):verifyable_object 
(PTHREAD_MUTEX_MAGIC)
+pthread_mutex::pthread_mutex (pthread_mutexattr *attr) :
+  verifyable_object (PTHREAD_MUTEX_MAGIC),
+  lock_counter (MUTEX_LOCK_COUNTER_INITIAL),
+  recursion_counter (0), win32_obj_id(NULL),
+  condwaits (0), owner (NULL), type (PTHREAD_MUTEX_DEFAULT),
+  pshared(PTHREAD_PROCESS_PRIVATE)
 {
-  /*attr checked in the C call */
-  if (attr && attr->pshared == PTHREAD_PROCESS_SHARED)
+  win32_obj_id = ::CreateSemaphore (&sec_none_nih, 0, LONG_MAX, NULL);
+  if (!win32_obj_id)
     {
-      // fail
       magic = 0;
       return;
     }
-  if (wincap.has_try_enter_critical_section ())
-    InitializeCriticalSection (&criticalsection);
-  else
+  /*attr checked in the C call */
+  if (attr)
     {
-      this->win32_obj_id = ::CreateMutex (&sec_none_nih, false, NULL);
-      if (!win32_obj_id)
-       magic = 0;
+      if (attr->pshared == PTHREAD_PROCESS_SHARED)
+        {
+          // fail
+          magic = 0;
+          return;
+        }
+
+      type = attr->mutextype;
     }
-  condwaits = 0;
-  pshared = PTHREAD_PROCESS_PRIVATE;
+
   /* threadsafe addition is easy */
   next = (pthread_mutex *) InterlockedExchangePointer (&MT_INTERFACE->mutexs, this);
 }
 
 pthread_mutex::~pthread_mutex ()
 {
-  if (wincap.has_try_enter_critical_section ())
-    DeleteCriticalSection (&criticalsection);
-  else
-    {
-      if (win32_obj_id)
-       CloseHandle (win32_obj_id);
-      win32_obj_id = NULL;
-    }
+  if (win32_obj_id)
+    CloseHandle (win32_obj_id);
   /* I'm not 100% sure the next bit is threadsafe. I think it is... */
   if (MT_INTERFACE->mutexs == this)
     /* TODO: printf an error if the return value != this */
@@ -1083,7 +1084,7 @@ pthread_mutex::~pthread_mutex ()
     {
       pthread_mutex *tempmutex = MT_INTERFACE->mutexs;
       while (tempmutex->next && tempmutex->next != this)
-       tempmutex = tempmutex->next;
+        tempmutex = tempmutex->next;
       /* but there may be a race between the loop above and this statement */
       /* TODO: printf an error if the return value != this */
       InterlockedExchangePointer (&tempmutex->next, this->next);
@@ -1093,34 +1094,92 @@ pthread_mutex::~pthread_mutex ()
 int
 pthread_mutex::Lock ()
 {
-  if (wincap.has_try_enter_critical_section ())
+  int result = 0;
+  pthread_t self = pthread::self ();
+
+  if (0 == InterlockedIncrement (&lock_counter))
+    SetOwner ();
+  else if (__pthread_equal(&owner, &self))
+    {
+      InterlockedDecrement (&lock_counter);
+      if (PTHREAD_MUTEX_RECURSIVE == type)
+        if (UINT_MAX != recursion_counter)
+          ++recursion_counter;
+        else
+          result = EAGAIN;
+      else
+        result = EDEADLK;
+    }
+  else
     {
-      EnterCriticalSection (&criticalsection);
-      return 0;
+      WaitForSingleObject (win32_obj_id, INFINITE);
+      SetOwner ();
     }
-  /* FIXME: Return 0 on success */
-  WaitForSingleObject (win32_obj_id, INFINITE);
-  return 0;
+
+  return result;
 }
 
-/* returns non-zero on failure */
 int
 pthread_mutex::TryLock ()
 {
-  if (wincap.has_try_enter_critical_section ())
-    return (!TryEnterCriticalSection (&criticalsection));
-  return (WaitForSingleObject (win32_obj_id, 0) == WAIT_TIMEOUT);
+  int result = 0;
+  pthread_t self = pthread::self ();
+
+  if (MUTEX_LOCK_COUNTER_INITIAL ==
+      InterlockedCompareExchange (&lock_counter, 0, MUTEX_LOCK_COUNTER_INITIAL ))
+    SetOwner ();
+  else if (__pthread_equal (&owner, &self) && PTHREAD_MUTEX_RECURSIVE == type)
+    if (UINT_MAX != recursion_counter)
+      ++recursion_counter;
+    else
+      result = EAGAIN;
+  else
+    result = EBUSY;
+
+  return result;
 }
 
 int
 pthread_mutex::UnLock ()
 {
-  if (wincap.has_try_enter_critical_section ())
+  pthread_t self = pthread::self ();
+
+  if (!__pthread_equal (&owner, &self))
+    return EPERM;
+
+  if (0 == --recursion_counter)
     {
-      LeaveCriticalSection (&criticalsection);
-      return 0;
+      owner = NULL;
+      if (MUTEX_LOCK_COUNTER_INITIAL != InterlockedDecrement (&lock_counter))
+        // Another thread is waiting
+        ::ReleaseSemaphore (win32_obj_id, 1, NULL);
     }
-  return (!ReleaseMutex (win32_obj_id));
+
+  return 0;
+}
+
+int
+pthread_mutex::Destroy ()
+{
+  if (condwaits || TryLock ())
+    // Do not destroy a condwaited or locked mutex
+    return EBUSY;
+  else if (recursion_counter != 1)
+    {
+      // Do not destroy a recursive locked mutex
+      --recursion_counter;
+      return EBUSY;
+    }
+
+  delete this;
+  return 0;
+}
+
+void
+pthread_mutex::SetOwner ()
+{
+   recursion_counter = 1;
+   owner = pthread::self ();
 }
 
 void
@@ -1129,15 +1188,15 @@ pthread_mutex::fixup_after_fork ()
   debug_printf ("mutex %x in fixup_after_fork", this);
   if (pshared != PTHREAD_PROCESS_PRIVATE)
     api_fatal ("pthread_mutex::fixup_after_fork () doesn'tunderstand PROCESS_SHARED 
mutex's");
-  /* FIXME: duplicate code here and in the constructor. */
-  if (wincap.has_try_enter_critical_section ())
-    InitializeCriticalSection (&criticalsection);
-  else
-    {
-      win32_obj_id = ::CreateMutex (&sec_none_nih, false, NULL);
-      if (!win32_obj_id)
-       api_fatal ("pthread_mutex::fixup_after_fork () failed to create new win32 
mutex");
-    }
+
+  // State is preserved with one exception : All waiting threads are gone after fork
+  if (lock_counter != MUTEX_LOCK_COUNTER_INITIAL)
+    lock_counter = 0;
+
+  win32_obj_id = ::CreateSemaphore (&sec_none_nih, 0, LONG_MAX, NULL);
+  if (!win32_obj_id)
+    api_fatal ("failed to recreate win32 semaphore for mutex");
+
 #if DETECT_BAD_APPS
   if (condwaits)
     api_fatal ("Forked () while a mutex has condition variables waiting on 
it.\nReport to [EMAIL PROTECTED]");
@@ -2184,16 +2243,17 @@ __pthread_mutex_unlock (pthread_mutex_t 
 int
 __pthread_mutex_destroy (pthread_mutex_t *mutex)
 {
+  int rv;
+
   if (check_valid_pointer (mutex) && (*mutex == PTHREAD_MUTEX_INITIALIZER))
     return 0;
   if (verifyable_object_isvalid (mutex, PTHREAD_MUTEX_MAGIC) != VALID_OBJECT)
     return EINVAL;
 
-  /*reading a word is atomic */
-  if ((*mutex)->condwaits)
-    return EBUSY;
+  rv = (*mutex)->Destroy ();
+  if (rv)
+    return rv;
 
-  delete (*mutex);
   *mutex = NULL;
   return 0;
 }
@@ -2227,10 +2287,6 @@ __pthread_mutexattr_getpshared (const pt
   return 0;
 }
 
-/*Win32 mutex's are equivalent to posix RECURSIVE mutexs.
- *We need to put glue in place to support other types of mutex's. We map
- *PTHREAD_MUTEX_DEFAULT to PTHREAD_MUTEX_RECURSIVE and return EINVAL for other types.
- */
 int
 __pthread_mutexattr_gettype (const pthread_mutexattr_t *attr, int *type)
 {
@@ -2240,10 +2296,7 @@ __pthread_mutexattr_gettype (const pthre
   return 0;
 }
 
-/*Currently pthread_mutex_init ignores the attr variable, this is because
- *none of the variables have any impact on it's behaviour.
- *
- *FIXME: write and test process shared mutex's.
+/*FIXME: write and test process shared mutex's.
  */
 int
 __pthread_mutexattr_init (pthread_mutexattr_t *attr)
@@ -2314,15 +2367,21 @@ __pthread_mutexattr_setpshared (pthread_
   return 0;
 }
 
-/*see __pthread_mutex_gettype */
 int
 __pthread_mutexattr_settype (pthread_mutexattr_t *attr, int type)
 {
   if (verifyable_object_isvalid (attr, PTHREAD_MUTEXATTR_MAGIC) != VALID_OBJECT)
     return EINVAL;
-  if (type != PTHREAD_MUTEX_RECURSIVE)
-    return EINVAL;
-  (*attr)->mutextype = type;
+
+  switch (type)
+    {
+    case PTHREAD_MUTEX_ERRORCHECK:
+    case PTHREAD_MUTEX_RECURSIVE:
+      (*attr)->mutextype = type;
+      break;
+    default:
+      return EINVAL;
+    }
   return 0;
 }
 
diff -urp src.old/winsup/cygwin/thread.h src/winsup/cygwin/thread.h
--- src.old/winsup/cygwin/thread.h      Wed Aug 14 14:31:13 2002
+++ src/winsup/cygwin/thread.h  Wed Aug 14 14:32:08 2002
@@ -152,6 +152,8 @@ private:
 #define SEM_MAGIC PTHREAD_MAGIC+7
 #define PTHREAD_ONCE_MAGIC PTHREAD_MAGIC+8;
 
+#define MUTEX_LOCK_COUNTER_INITIAL   (-1)
+
 /* verifyable_object should not be defined here - it's a general purpose class */
 
 class verifyable_object
@@ -217,15 +219,20 @@ public:
 class pthread_mutex:public verifyable_object
 {
 public:
-  CRITICAL_SECTION criticalsection;
+  LONG lock_counter;
+  unsigned int recursion_counter;
   HANDLE win32_obj_id;
   LONG condwaits;
+  pthread_t owner;
+  int type;
   int pshared;
   class pthread_mutex * next;
 
   int Lock ();
   int TryLock ();
   int UnLock ();
+  int Destroy ();
+  void SetOwner ();
   void fixup_after_fork ();
 
   pthread_mutex (pthread_mutexattr * = NULL);

Reply via email to