2001-02-09 Ross Johnson <rpj@setup1.ise.canberra.edu.au>exp-2001-02-10-passed

        * nonportable.c (pthread_mutex_setdefaulttype_np): New
        function for changing the default mutex type.

        * mutex.c (ptw32_InitializeCriticalSection): Removed.
        (ptw32_InitializeCriticalSection): Removed.
        (ptw32_InitializeCriticalSection): Removed.
        (ptw32_InitializeCriticalSection): Removed.
        (ptw32_InitializeCriticalSection): Removed.
        (pthread_mutex_init): Apply Thomas Pfaff's original
        patches; remove use of critical sections and adapt
        for different mutex types (see log entry for 2001-01-10).
        The disadvantage of using critical sections is that
        they don't appear to be sharable between processes.
        This implementation provides the opportunity to place
        mutex objects in shared memory. Thread priority
        determines the order in which waiting threads acquire
        the mutex although the implementation includes a
        mechanism to prevent threads hogging the mutex through
        successive unlock/lock operations.
        (pthread_mutex_destroy): Likewise.
        (pthread_mutex_lock): Likewise.
        (pthread_mutex_unlock): Likewise.
        (pthread_mutex_trylock): Likewise.

        * pthread.h (rand_r): Add redundant test of '_seed' arg
        to avoid "unused variable" warnings. Any good compiler
        will optimise the test away anyway.

8 files changed, 375 insertions, 74 deletions

diff --git a/ANNOUNCE b/ANNOUNCE
index d2def8b..83977ac 100644
--- a/ANNOUNCE
+++ b/ANNOUNCE
@@ -111,7 +111,14 @@ Known bugs in this snapshot
 3. This is an interim snapshot as there are still some additional

    patches to go in, eg. to fix problems with errno support under

    some circumstances. Some people are seeing compile warnings

-   to do with _errno.

+   to do with _errno. Sorry if this affects you but applying

+   people's suggested patches is causing compiles to fail for me. As

+   I believe this affects ports such as WinCE that I don't have

+   the tools for, I'd appreciate it if someone could give me

+   a definitive patch that works everywhere.

+

+4. Turning on /Ox optimisation in the VC++ compiler causes an

+   "invalid page fault" to be generated in the test condvar2.c.

 

 Caveats

 -------

@@ -206,7 +213,6 @@ The following functions are implemented:
                                         PTHREAD_MUTEX_NORMAL

                                         PTHREAD_MUTEX_ERRORCHECK

                                         PTHREAD_MUTEX_RECURSIVE  )

-

       pthread_mutex_init

       pthread_mutex_destroy

       pthread_mutex_lock

@@ -275,9 +281,9 @@ The following functions are implemented:
       ---------------------------

       Non-portable routines (see the README.NONPORTABLE file for usage)

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

-      pthread_mutexattr_setforcecs_np

       pthread_getw32threadhandle_np

       pthread_delay_np

+      pthread_mutex_setdefaulttype_np

       pthread_win32_process_attach_np

       pthread_win32_process_detach_np

       pthread_win32_thread_attach_np

@@ -393,13 +399,15 @@ Application Development Environments
 ------------------------------------

       

 MSVC:

-MSVC using SEH works.

-MSVC using C++ EH works.

+MSVC using SEH works. Distribute pthreadVSE.dll with your application.

+MSVC using C++ EH works. Distribute pthreadVCE.dll with your application.

 

 Mingw32:

 You need gcc-2.95.2-1 modified as per pthreads-win32 FAQ answer (6), with

 binutils-19990818-1 and msvcrt runtime-2000-03-27. Mingw32 must use

-the thread-safe MSVCRT library (see the FAQ).

+the thread-safe MSVCRT library (see the FAQ). You need to distribute

+the gcc.dll DLL from Mingw32 with your application (as well as

+pthreadGCE.dll of course).

 

 Cygwin: (http://sourceware.cygnus.com/cygwin/)

 Cygwin aims to provide a complete POSIX environment on top of Win32, including

@@ -424,7 +432,7 @@ For convenience, the following pre-built files are available on the FTP site
 	pthreadVCE.lib

 	pthreadVSE.dll	- built with MSVC compiler using SEH

 	pthreadVSE.lib

-	pthreadGCE.dll	- built with Mingw32 G++

+	pthreadGCE.dll	- built with Mingw32 G++ 2.95.2-1 

 	libpthreadw32.a	- derived from pthreadGCE.dll

 

 These are the only files you need in order to build POSIX threads

diff --git a/ChangeLog b/ChangeLog
index 5a8c6a6..ea83e1c 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -9,14 +9,25 @@
 	(ptw32_InitializeCriticalSection): Removed.

 	(ptw32_InitializeCriticalSection): Removed.

 	(pthread_mutex_init): Apply Thomas Pfaff's original

-	patches but altered slightly to avoid using

-	critical sections and retain/adapt for different

-	mutex types (see log entry for 2001-01-10).

+	patches; remove use of critical sections and adapt

+	for different mutex types (see log entry for 2001-01-10).

+	The disadvantage of using critical sections is that

+	they don't appear to be sharable between processes.

+	This implementation provides the opportunity to place

+	mutex objects in shared memory. Thread priority

+	determines the order in which waiting threads acquire

+	the mutex although the implementation includes a

+	mechanism to prevent threads hogging the mutex through

+	successive unlock/lock operations.

 	(pthread_mutex_destroy): Likewise.

 	(pthread_mutex_lock): Likewise.

 	(pthread_mutex_unlock): Likewise.

 	(pthread_mutex_trylock): Likewise.

 

+	* pthread.h (rand_r): Add redundant test of '_seed' arg

+	to avoid "unused variable" warnings. Any good compiler

+	will optimise the test away anyway.

+

 	* Tagged repository 'exp-2001-02-09-passed'.

 	

 2001-02-09  Ross Johnson  <rpj@special.ise.canberra.edu.au>

diff --git a/implement.h b/implement.h
index a1a52f8..8297a46 100644
--- a/implement.h
+++ b/implement.h
@@ -127,11 +127,14 @@ struct pthread_mutexattr_t_ {
 };
 
 struct pthread_mutex_t_ {
-  int lock_idx;
-  int try_lock;
+  long lock_idx;
+  long try_lock;
   int pshared;
   int type;
   pthread_t owner;
+  long waiters;                      /* These last elements ensure fairness */
+  pthread_t lastOwner;               /* and guard against canceled threads. */
+  pthread_t lastWaiter;
 };
 
 struct pthread_key_t_ {
diff --git a/mutex.c b/mutex.c
index f99ce86..d1f94b7 100644
--- a/mutex.c
+++ b/mutex.c
@@ -87,6 +87,32 @@ ptw32_mutex_check_need_init(pthread_mutex_t *mutex)
 
 int
 pthread_mutex_init(pthread_mutex_t *mutex, const pthread_mutexattr_t *attr)
+     /*
+      * ------------------------------------------------------
+      * DOCPUBLIC
+      *      Initializes a mutex object with supplied or
+      *      default attributes.
+      *
+      * PARAMETERS
+      *      mutex
+      *              pointer to an instance of pthread_mutex_t
+      *      attr
+      *              pointer to an instance of pthread_mutexattr_t
+      *
+      *
+      * DESCRIPTION
+      *      Initializes a mutex object with supplied or
+      *      default attributes.
+      *
+      * RESULTS
+      *              0               successfully initialized attr,
+      *              EINVAL          not a valid mutex pointer,
+      *              ENOMEM          insufficient memory for attr,
+      *              ENOSYS          one or more requested attributes
+      *                              are not supported.
+      *
+      * ------------------------------------------------------
+      */
 {
   int result = 0;
   pthread_mutex_t mx;
@@ -104,8 +130,11 @@ pthread_mutex_init(pthread_mutex_t *mutex, const pthread_mutexattr_t *attr)
     }
 
   mx->lock_idx = -1;
-  mx->owner = NULL;
   mx->try_lock = 0;
+  mx->owner = NULL;
+  mx->waiters = 0;
+  mx->lastOwner = NULL;
+  mx->lastWaiter = NULL;
 
   if (attr != NULL && *attr != NULL)
     {
@@ -159,6 +188,27 @@ FAIL0:
 
 int
 pthread_mutex_destroy(pthread_mutex_t *mutex)
+     /*
+      * ------------------------------------------------------
+      * DOCPUBLIC
+      *      Destroys a mutex object and returns any resources
+      *      to the system.
+      *
+      * PARAMETERS
+      *      mutex
+      *              pointer to an instance of pthread_mutex_t
+      *
+      * DESCRIPTION
+      *      Destroys a mutex object and returns any resources
+      *      to the system.
+      *
+      * RESULTS
+      *              0               successfully initialized attr,
+      *              EINVAL          not a valid mutex pointer,
+      *              EBUSY           the mutex is currently locked.
+      *
+      * ------------------------------------------------------
+      */
 {
   int result = 0;
   pthread_mutex_t mx;
@@ -497,6 +547,12 @@ pthread_mutexattr_settype (pthread_mutexattr_t * attr,
      /*
       * ------------------------------------------------------
       *
+      * DOCPUBLIC
+      * The pthread_mutexattr_settype() and
+      * pthread_mutexattr_gettype() functions  respectively set and
+      * get the mutex type  attribute. This attribute is set in  the
+      * type parameter to these functions.
+      *
       * PARAMETERS
       *      attr
       *              pointer to an instance of pthread_mutexattr_t
@@ -513,9 +569,9 @@ pthread_mutexattr_settype (pthread_mutexattr_t * attr,
       *                      PTHREAD_MUTEX_RECURSIVE
       *
       * DESCRIPTION
-      * The pthread_mutexattr_gettype() and
-      * pthread_mutexattr_settype() functions  respectively get and
-      * set the mutex type  attribute. This attribute is set in  the
+      * The pthread_mutexattr_settype() and
+      * pthread_mutexattr_gettype() functions  respectively set and
+      * get the mutex type  attribute. This attribute is set in  the
       * type  parameter to these functions. The default value of the
       * type  attribute is  PTHREAD_MUTEX_DEFAULT.
       * 
@@ -614,6 +670,38 @@ pthread_mutexattr_gettype (pthread_mutexattr_t * attr,
 
 int
 pthread_mutex_lock(pthread_mutex_t *mutex)
+     /*
+      * ------------------------------------------------------
+      * DOCPUBLIC
+      *      Locks an unlocked mutex. If the mutex is already
+      *      locked, the calling thread usually blocks, but
+      *      depending on the current owner and type of the mutex
+      *      may recursively lock the mutex or return an error.
+      *
+      * PARAMETERS
+      *      mutex
+      *              pointer to an instance of pthread_mutex_t
+      *
+      * DESCRIPTION
+      *      Locks an unlocked mutex. If the mutex is already
+      *      locked, the calling thread usually blocks, but
+      *      depending on the current owner and type of the mutex
+      *      may recursively lock the mutex or return an error.
+      *
+      *      See the description under pthread_mutexattr_settype()
+      *      for details.
+      *
+      * RESULTS
+      *              0               successfully locked mutex,
+      *              EINVAL          not a valid mutex pointer,
+      *              ENOMEM          insufficient memory to initialise
+      *                              the statically declared mutex object,
+      *              EDEADLK         the mutex is of type
+      *                              PTHREAD_MUTEX_ERRORCHECK and the
+      *                              calling thread already owns the mutex.
+      *
+      * ------------------------------------------------------
+      */
 {
   int result = 0;
   pthread_mutex_t mx;
@@ -644,57 +732,196 @@ pthread_mutex_lock(pthread_mutex_t *mutex)
         {
         case PTHREAD_MUTEX_DEFAULT:
         case PTHREAD_MUTEX_RECURSIVE:
-          if (InterlockedIncrement(&mx->lock_idx) > 0)
+          while (TRUE)
             {
-              while (mx->try_lock)
+              if (0 == InterlockedIncrement(&mx->lock_idx))
                 {
-                  Sleep(0);
+                  /*
+                   * Ensure that we give other waiting threads a
+                   * chance to take the mutex if we held it last time.
+                   */
+                  if (InterlockedIncrement(&mx->waiters) > 0
+                      && mx->lastOwner == self)
+                    {
+                      /*
+                       * Check to see if other waiting threads
+                       * have stopped waiting but haven't decremented
+                       * the 'waiters' counter - ie. they may have been
+                       * canceled. If we're wrong then waiting threads will
+                       * increment the value again.
+                       */
+                      if (mx->lastWaiter == self)
+                        {
+                          (void) InterlockedExchange(&mx->waiters, 0L);
+                        }
+                      else
+                        {
+                          goto WAIT_RECURSIVE;
+                        }
+                    }
+LOCK_RECURSIVE:
+                  /*
+                   * Take the lock.
+                   */
+                  mx->owner = self;
+                  mx->lastOwner = self;
+                  mx->lastWaiter = NULL;
+                  break;
                 }
-
-              while (mx->lock_idx > 0 && mx->owner != self)
+              else
                 {
+                  while (mx->try_lock)
+                    {
+                      Sleep(0);
+                    }
+
+                  if (mx->owner == self)
+                    {
+                      goto LOCK_RECURSIVE;
+                    }
+WAIT_RECURSIVE:
+                  InterlockedIncrement(&mx->waiters);
+                  mx->lastWaiter = self;
+                  InterlockedDecrement(&mx->lock_idx);
                   Sleep(0);
+                  /*
+                   * Thread priorities may have tricked another
+                   * thread into thinking we weren't waiting anymore.
+                   * If so, waiters will equal 0 so set it to 1
+                   * before we decrement it.
+                   */
+                  if (InterlockedDecrement(&mx->waiters) < 0)
+                    {
+                      InterlockedExchange(&mx->waiters, 0);
+                    }
                 }
             }
-          mx->owner = self;
           break;
         case PTHREAD_MUTEX_NORMAL:
           /*
            * If the thread already owns the mutex
            * then the thread will become deadlocked.
            */
-          while (InterlockedIncrement(&mx->lock_idx) > 0)
-            {
-              InterlockedDecrement(&mx->lock_idx);
-              Sleep(0);
-            }
-          mx->owner = self;
-          break;
-        case PTHREAD_MUTEX_ERRORCHECK:
-          if (0 == InterlockedIncrement(&mx->lock_idx))
-            {
-              mx-owner = self;
-            }
-          else
+          while (TRUE)
             {
-              while (mx->try_lock)
+              if (0 == InterlockedIncrement(&mx->lock_idx))
                 {
-                  Sleep(0);
+                  /*
+                   * Ensure that we give other waiting threads a
+                   * chance to take the mutex if we held it last time.
+                   */
+                  if (InterlockedIncrement(&mx->waiters) > 0
+                      && mx->lastOwner == self)
+                    {
+                      /*
+                       * Check to see if other waiting threads
+                       * have stopped waiting but haven't decremented
+                       * the 'waiters' counter - ie. they may have been
+                       * canceled.
+                       */
+                      if (mx->lastWaiter == self)
+                        {
+                          InterlockedExchange(&mx->waiters, 0L);
+                        }
+                      else
+                        {
+                          goto WAIT_NORMAL;
+                        }
+                    }
+                  /*
+                   * Take the lock.
+                   */
+                  mx->owner = self;
+                  mx->lastOwner = self;
+                  mx->lastWaiter = NULL;
+                  break;
                 }
-
-              while (mx->lock_idx > 0 && mx->owner != self)
+              else
                 {
+                  while (mx->try_lock)
+                    {
+                      Sleep(0);
+                    }
+WAIT_NORMAL:
+                  InterlockedIncrement(&mx->waiters);
+                  mx->lastWaiter = self;
+                  InterlockedDecrement(&mx->lock_idx);
                   Sleep(0);
+                  /*
+                   * Thread priorities may have tricked another
+                   * thread into thinking we weren't waiting anymore.
+                   * If so, waiters will equal 0 so set it to 1
+                   * before we decrement it.
+                   */
+                  if (InterlockedDecrement(&mx->waiters) < 0)
+                    {
+                      InterlockedExchange(&mx->waiters, 0);
+                    }
                 }
-
-              if (mx->owner == self)
+            }
+          break;
+        case PTHREAD_MUTEX_ERRORCHECK:
+          while (TRUE)
+            {
+              if (0 == InterlockedIncrement(&mx->lock_idx))
                 {
-                  InterlockedDecrement(&mx->lock_idx);
-                  result = EDEADLK;
+                  /*
+                   * Ensure that we give other waiting threads a
+                   * chance to take the mutex if we held it last time.
+                   */
+                  if (InterlockedIncrement(&mx->waiters) > 0
+                      && mx->lastOwner == self)
+                    {
+                      /*
+                       * Check to see if other waiting threads
+                       * have stopped waiting but haven't decremented
+                       * the 'waiters' counter - ie. they may have been
+                       * canceled.
+                       */
+                      if (mx->lastWaiter == self)
+                        {
+                          InterlockedExchange(&mx->waiters, 0L);
+                        }
+                      else
+                        {
+                          goto WAIT_ERRORCHECK;
+                        }
+                    }
+                  /*
+                   * Take the lock.
+                   */
+                  mx->owner = self;
+                  mx->lastOwner = self;
+                  mx->lastWaiter = NULL;
+                  break;
                 }
               else
                 {
-                  mx->owner = self;
+                  while (mx->try_lock)
+                    {
+                      Sleep(0);
+                    }
+
+                  if (mx->owner == self)
+                    {
+                      result = EDEADLK;
+                      break;
+                    }
+WAIT_ERRORCHECK:
+                  InterlockedIncrement(&mx->waiters);
+                  mx->lastWaiter = self;
+                  InterlockedDecrement(&mx->lock_idx);
+                  Sleep(0);
+                  /*
+                   * Thread priorities may have tricked another
+                   * thread into thinking we weren't waiting anymore.
+                   * If so, waiters will equal 0 so set it to 1
+                   * before we decrement it.
+                   */
+                  if (InterlockedDecrement(&mx->waiters) < 0)
+                    {
+                      InterlockedExchange(&mx->waiters, 0);
+                    }
                 }
             }
           break;
@@ -709,6 +936,32 @@ pthread_mutex_lock(pthread_mutex_t *mutex)
 
 int
 pthread_mutex_unlock(pthread_mutex_t *mutex)
+     /*
+      * ------------------------------------------------------
+      * DOCPUBLIC
+      *      Decrements the lock count of the currently locked mutex.
+      *
+      * PARAMETERS
+      *      mutex
+      *              pointer to an instance of pthread_mutex_t
+      *
+      * DESCRIPTION
+      *      Decrements the lock count of the currently locked mutex.
+      *
+      *      If the count reaches it's 'unlocked' value then it
+      *      is available to be locked by another waiting thread.
+      *      The implementation ensures that other waiting threads
+      *      get a chance to take the unlocked mutex before the unlocking
+      *      thread can re-lock it.
+      *
+      * RESULTS
+      *              0               successfully locked mutex,
+      *              EINVAL          not a valid mutex pointer,
+      *              EPERM           the current thread does not own
+      *                              the mutex.
+      *
+      * ------------------------------------------------------
+      */
 {
   int result = 0;
   pthread_mutex_t mx;
@@ -725,35 +978,29 @@ pthread_mutex_unlock(pthread_mutex_t *mutex)
    * race condition. If another thread holds the
    * lock then we shouldn't be in here.
    */
-  if (mx != (pthread_mutex_t) PTW32_OBJECT_AUTO_INIT)
+  if (mx != (pthread_mutex_t) PTW32_OBJECT_AUTO_INIT
+      && mx->owner == pthread_self())
     {
-      if (mx->owner == pthread_self())
-	{
-          switch (mx->type)
+      switch (mx->type)
+        {
+        case PTHREAD_MUTEX_NORMAL:
+        case PTHREAD_MUTEX_ERRORCHECK:
+          mx->owner = NULL;
+          break;
+        case PTHREAD_MUTEX_RECURSIVE:
+        default:
+          if (mx->lock_idx == 0)
             {
-            case PTHREAD_MUTEX_NORMAL:
-            case PTHREAD_MUTEX_ERRORCHECK:
               mx->owner = NULL;
-              break;
-            case PTHREAD_MUTEX_RECURSIVE:
-            default:
-              if (mx->lock_idx == 0)
-                {
-                  mx->owner = NULL;
-                }
-              break;
             }
-	      
-          InterlockedDecrement(&mx->lock_idx);
+          break;
         }
-      else
-	{
-	  result = EPERM;
-	}
+
+      InterlockedDecrement(&mx->lock_idx);
     }
   else
     {
-      result = EINVAL;
+      result = EPERM;
     }
 
   return result;
@@ -761,6 +1008,35 @@ pthread_mutex_unlock(pthread_mutex_t *mutex)
 
 int
 pthread_mutex_trylock(pthread_mutex_t *mutex)
+     /*
+      * ------------------------------------------------------
+      * DOCPUBLIC
+      *      Tries to lock a mutex. If the mutex is already
+      *      locked (by any thread, including the calling thread),
+      *      the calling thread returns without waiting
+      *      for the mutex to be freed (nor recursively locking
+      *      the mutex if the calling thread currently owns it).
+      *
+      * PARAMETERS
+      *      mutex
+      *              pointer to an instance of pthread_mutex_t
+      *
+      * DESCRIPTION
+      *      Tries to lock a mutex. If the mutex is already
+      *      locked (by any thread, including the calling thread),
+      *      the calling thread returns without waiting
+      *      for the mutex to be freed (nor recursively locking
+      *      the mutex if the calling thread currently owns it).
+      *
+      * RESULTS
+      *              0               successfully locked the mutex,
+      *              EINVAL          not a valid mutex pointer,
+      *              EBUSY           the mutex is currently locked,
+      *              ENOMEM          insufficient memory to initialise
+      *                              the statically declared mutex object.
+      *
+      * ------------------------------------------------------
+      */
 {
   int result = 0;
   pthread_mutex_t mx;
@@ -800,14 +1076,13 @@ pthread_mutex_trylock(pthread_mutex_t *mutex)
           if (0 == InterlockedIncrement(&mx->lock_idx))
             {
               mx->owner = self;
+              mx->lastOwner = self;
+              mx->lastWaiter = NULL;
             }
           else
             {
               InterlockedDecrement(&mx->lock_idx);
-              if (mx->owner == self)
-                {
-                  result = EBUSY;
-                }
+              result = EBUSY;
             }
 
           mx->try_lock--;
diff --git a/nonportable.c b/nonportable.c
index 27b67df..f442c44 100644
--- a/nonportable.c
+++ b/nonportable.c
@@ -287,7 +287,7 @@ pthread_mutex_setdefaulttype_np (int newtype, int * oldtype)
 
   if (oldtype != NULL)
     {
-      *oldType = ptw32_mutex_mapped_default;
+      *oldtype = ptw32_mutex_mapped_default;
     }
 
   switch (newtype)
diff --git a/pthread.h b/pthread.h
index e40aaef..176892b 100644
--- a/pthread.h
+++ b/pthread.h
@@ -887,8 +887,9 @@ int * _errno( void );
 	( *(_result) = *localtime( (_clock) ), \
 	  (_result) )
 
+/* The redundant test avoids unused variable warnings */
 #define rand_r( _seed ) \
-	rand()
+      ((seed == seed) ? rand() : rand())
 
 
 #ifdef __cplusplus
diff --git a/tests/cancel2.c b/tests/cancel2.c
index 3a7c2c5..f1b5106 100644
--- a/tests/cancel2.c
+++ b/tests/cancel2.c
@@ -138,6 +138,7 @@ main()
 {
   int failed = 0;
   int i;
+  int ret;
   pthread_t t[NUMTHREADS + 1];
 
   assert((t[0] = pthread_self()) != NULL);
@@ -155,7 +156,9 @@ main()
    */
   Sleep(500);
 
-  assert(pthread_mutex_unlock(&waitLock) == 0);
+  ret = pthread_mutex_unlock(&waitLock);
+  assert(ret != EPERM);
+  assert(ret == 0);
 
   Sleep(500);
 
diff --git a/tests/rwlock7.c b/tests/rwlock7.c
index cf20bfd..ff2db21 100644
--- a/tests/rwlock7.c
+++ b/tests/rwlock7.c
@@ -10,7 +10,7 @@
 
 #define THREADS         5
 #define DATASIZE        15
-#define ITERATIONS      10000
+#define ITERATIONS      1000000
 
 /*
  * Keep statistics for each thread.
@@ -162,7 +162,7 @@ main (int argc, char *argv[])
 
   _ftime(&currSysTime2);
 
-  printf( "\nstart: %d/%d, stop: %d/%d, duration:%d\n",
+  printf( "\nstart: %ld/%d, stop: %ld/%d, duration:%ld\n",
           currSysTime1.time,currSysTime1.millitm,
           currSysTime2.time,currSysTime2.millitm,
           (currSysTime2.time*1000+currSysTime2.millitm) -