Replace global Critical Sections with MCS Queue locks

ChangeLog

@@ -1,3 +1,26 @@
+2011-03-09  Ross Johnson <ross.johnson at homemail.com.au>
+
+	* implement.h (ptw32_thread_t_): Add process unique sequence number.
+	* global.c: Replace global Critical Section objects with MCS
+	queue locks.
+	* implement.h: Likewise.
+	* pthread_cond_destroy.c: Likewise.
+	* pthread_cond_init.c: Likewise.
+	* pthread_detach.c: Likewise.
+	* pthread_join.c: Likewise.
+	* pthread_kill.c: Likewise.
+	* pthread_mutex_destroy.c: Likewise.
+	* pthread_rwlock_destroy.c: Likewise.
+	* pthread_spin_destroy.c: Likewise.
+	* pthread_timechange_handler_np.c: Likewise.
+	* ptw32_cond_check_need_init.c: Likewise.
+	* ptw32_mutex_check_need_init.c: Likewise.
+	* ptw32_processInitialize.c: Likewise.
+	* ptw32_processTerminate.c: Likewise.
+	* ptw32_reuse.c: Likewise.
+	* ptw32_rwlock_check_need_init.c: Likewise.
+	* ptw32_spinlock_check_need_init.c: Likewise.
+
 2011-03-06  Ross Johnson <ross.johnson at homemail.com.au>
 
 	* several (MINGW64): Cast and call fixups for 64 bit compatibility;

global.c

@@ -62,37 +62,37 @@ DWORD (*ptw32_register_cancelation) (PAPCFUNC, HANDLE, DWORD) = NULL;
 /*
  * Global lock for managing pthread_t struct reuse.
  */
-CRITICAL_SECTION ptw32_thread_reuse_lock;
+ptw32_mcs_lock_t ptw32_thread_reuse_lock;
 
 /*
  * Global lock for testing internal state of statically declared mutexes.
  */
-CRITICAL_SECTION ptw32_mutex_test_init_lock;
+ptw32_mcs_lock_t ptw32_mutex_test_init_lock;
 
 /*
  * Global lock for testing internal state of PTHREAD_COND_INITIALIZER
  * created condition variables.
  */
-CRITICAL_SECTION ptw32_cond_test_init_lock;
+ptw32_mcs_lock_t ptw32_cond_test_init_lock;
 
 /*
  * Global lock for testing internal state of PTHREAD_RWLOCK_INITIALIZER
  * created read/write locks.
  */
-CRITICAL_SECTION ptw32_rwlock_test_init_lock;
+ptw32_mcs_lock_t ptw32_rwlock_test_init_lock;
 
 /*
  * Global lock for testing internal state of PTHREAD_SPINLOCK_INITIALIZER
  * created spin locks.
  */
-CRITICAL_SECTION ptw32_spinlock_test_init_lock;
+ptw32_mcs_lock_t ptw32_spinlock_test_init_lock;
 
 /*
  * Global lock for condition variable linked list. The list exists
  * to wake up CVs when a WM_TIMECHANGE message arrives. See
  * w32_TimeChangeHandler.c.
  */
-CRITICAL_SECTION ptw32_cond_list_lock;
+ptw32_mcs_lock_t ptw32_cond_list_lock;
 
 #ifdef _UWIN
 /*

implement.h

@@ -131,7 +131,8 @@ struct ptw32_thread_t_
 #ifdef _UWIN
   DWORD dummy[5];
 #endif
-  DWORD thread;
+  UINT64 seqNumber;		/* Process-unique thread sequence number */
+  DWORD thread;			/* Win32 thread ID */
   HANDLE threadH;		/* Win32 thread handle - POSIX thread is invalid if threadH == 0 */
   pthread_t ptHandle;		/* This thread's permanent pthread_t handle */
   ptw32_thread_t * prevReuse;	/* Links threads on reuse stack */
@@ -535,12 +536,12 @@ extern int ptw32_concurrency;
 
 extern int ptw32_features;
 
-extern CRITICAL_SECTION ptw32_thread_reuse_lock;
-extern CRITICAL_SECTION ptw32_mutex_test_init_lock;
-extern CRITICAL_SECTION ptw32_cond_list_lock;
-extern CRITICAL_SECTION ptw32_cond_test_init_lock;
-extern CRITICAL_SECTION ptw32_rwlock_test_init_lock;
-extern CRITICAL_SECTION ptw32_spinlock_test_init_lock;
+extern ptw32_mcs_lock_t ptw32_thread_reuse_lock;
+extern ptw32_mcs_lock_t ptw32_mutex_test_init_lock;
+extern ptw32_mcs_lock_t ptw32_cond_list_lock;
+extern ptw32_mcs_lock_t ptw32_cond_test_init_lock;
+extern ptw32_mcs_lock_t ptw32_rwlock_test_init_lock;
+extern ptw32_mcs_lock_t ptw32_spinlock_test_init_lock;
 
 #ifdef _UWIN
 extern int pthread_count;

pthread_cond_destroy.c

@@ -126,7 +126,8 @@ pthread_cond_destroy (pthread_cond_t * cond)
 
   if (*cond != PTHREAD_COND_INITIALIZER)
     {
-      EnterCriticalSection (&ptw32_cond_list_lock);
+      ptw32_mcs_local_node_t node;
+      ptw32_mcs_lock_acquire(&ptw32_cond_list_lock, &node);
 
       cv = *cond;
 
@@ -154,7 +155,7 @@ pthread_cond_destroy (pthread_cond_t * cond)
 
       if (result != 0)
         {
-          LeaveCriticalSection (&ptw32_cond_list_lock);
+          ptw32_mcs_lock_release(&node);
           return result;
         }
 
@@ -213,14 +214,15 @@ pthread_cond_destroy (pthread_cond_t * cond)
 	  (void) free (cv);
 	}
 
-      LeaveCriticalSection (&ptw32_cond_list_lock);
+      ptw32_mcs_lock_release(&node);
     }
   else
     {
+      ptw32_mcs_local_node_t node;
       /*
        * See notes in ptw32_cond_check_need_init() above also.
        */
-      EnterCriticalSection (&ptw32_cond_test_init_lock);
+      ptw32_mcs_lock_acquire(&ptw32_cond_test_init_lock, &node);
 
       /*
        * Check again.
@@ -244,7 +246,7 @@ pthread_cond_destroy (pthread_cond_t * cond)
 	  result = EBUSY;
 	}
 
-      LeaveCriticalSection (&ptw32_cond_test_init_lock);
+      ptw32_mcs_lock_release(&node);
     }
 
   return ((result != 0) ? result : ((result1 != 0) ? result1 : result2));

pthread_cond_init.c

@@ -138,7 +138,9 @@ pthread_cond_init (pthread_cond_t * cond, const pthread_condattr_t * attr)
 DONE:
   if (0 == result)
     {
-      EnterCriticalSection (&ptw32_cond_list_lock);
+      ptw32_mcs_local_node_t node;
+
+      ptw32_mcs_lock_acquire(&ptw32_cond_list_lock, &node);
 
       cv->next = NULL;
       cv->prev = ptw32_cond_list_tail;
@@ -155,7 +157,7 @@ pthread_cond_init (pthread_cond_t * cond, const pthread_condattr_t * attr)
 	  ptw32_cond_list_head = cv;
 	}
 
-      LeaveCriticalSection (&ptw32_cond_list_lock);
+      ptw32_mcs_lock_release(&node);
     }
 
   *cond = cv;

pthread_detach.c

@@ -77,8 +77,9 @@ pthread_detach (pthread_t thread)
   int result;
   BOOL destroyIt = PTW32_FALSE;
   ptw32_thread_t * tp = (ptw32_thread_t *) thread.p;
+  ptw32_mcs_local_node_t node;
 
-  EnterCriticalSection (&ptw32_thread_reuse_lock);
+  ptw32_mcs_lock_acquire(&ptw32_thread_reuse_lock, &node);
 
   if (NULL == tp
       || thread.x != tp->ptHandle.x)
@@ -120,7 +121,7 @@ pthread_detach (pthread_t thread)
 	}
     }
 
-  LeaveCriticalSection (&ptw32_thread_reuse_lock);
+  ptw32_mcs_lock_release(&node);
 
   if (result == 0)
     {

pthread_join.c

@@ -85,8 +85,9 @@ pthread_join (pthread_t thread, void **value_ptr)
   int result;
   pthread_t self;
   ptw32_thread_t * tp = (ptw32_thread_t *) thread.p;
+  ptw32_mcs_local_node_t node;
 
-  EnterCriticalSection (&ptw32_thread_reuse_lock);
+  ptw32_mcs_lock_acquire(&ptw32_thread_reuse_lock, &node);
 
   if (NULL == tp
       || thread.x != tp->ptHandle.x)
@@ -102,7 +103,7 @@ pthread_join (pthread_t thread, void **value_ptr)
       result = 0;
     }
 
-  LeaveCriticalSection (&ptw32_thread_reuse_lock);
+  ptw32_mcs_lock_release(&node);
 
   if (result == 0)
     {

pthread_kill.c

@@ -77,8 +77,9 @@ pthread_kill (pthread_t thread, int sig)
 {
   int result = 0;
   ptw32_thread_t * tp;
+  ptw32_mcs_local_node_t node;
 
-  EnterCriticalSection (&ptw32_thread_reuse_lock);
+  ptw32_mcs_lock_acquire(&ptw32_thread_reuse_lock, &node);
 
   tp = (ptw32_thread_t *) thread.p;
 
@@ -89,7 +90,7 @@ pthread_kill (pthread_t thread, int sig)
       result = ESRCH;
     }
 
-  LeaveCriticalSection (&ptw32_thread_reuse_lock);
+  ptw32_mcs_lock_release(&node);
 
   if (0 == result && 0 != sig)
     {

pthread_mutex_destroy.c

@@ -71,10 +71,6 @@ pthread_mutex_destroy (pthread_mutex_t * mutex)
 	       * be too late invalidating the mutex below since another thread
 	       * may already have entered mutex_lock and the check for a valid
 	       * *mutex != NULL.
-	       *
-	       * Note that this would be an unusual situation because it is not
-	       * common that mutexes are destroyed while they are still in
-	       * use by other threads.
 	       */
 	      *mutex = NULL;
 
@@ -112,10 +108,13 @@ pthread_mutex_destroy (pthread_mutex_t * mutex)
     }
   else
     {
+      ptw32_mcs_local_node_t node;
+
       /*
        * See notes in ptw32_mutex_check_need_init() above also.
        */
-      EnterCriticalSection (&ptw32_mutex_test_init_lock);
+
+      ptw32_mcs_lock_acquire(&ptw32_mutex_test_init_lock, &node);
 
       /*
        * Check again.
@@ -138,8 +137,7 @@ pthread_mutex_destroy (pthread_mutex_t * mutex)
 	   */
 	  result = EBUSY;
 	}
-
-      LeaveCriticalSection (&ptw32_mutex_test_init_lock);
+      ptw32_mcs_lock_release(&node);
     }
 
   return (result);

pthread_rwlock_destroy.c

@@ -108,10 +108,11 @@ pthread_rwlock_destroy (pthread_rwlock_t * rwlock)
     }
   else
     {
+      ptw32_mcs_local_node_t node;
       /*
        * See notes in ptw32_rwlock_check_need_init() above also.
        */
-      EnterCriticalSection (&ptw32_rwlock_test_init_lock);
+      ptw32_mcs_lock_acquire(&ptw32_rwlock_test_init_lock, &node);
 
       /*
        * Check again.
@@ -135,7 +136,7 @@ pthread_rwlock_destroy (pthread_rwlock_t * rwlock)
 	  result = EBUSY;
 	}
 
-      LeaveCriticalSection (&ptw32_rwlock_test_init_lock);
+      ptw32_mcs_lock_release(&node);
     }
 
   return ((result != 0) ? result : ((result1 != 0) ? result1 : result2));

pthread_spin_destroy.c

@@ -81,7 +81,9 @@ pthread_spin_destroy (pthread_spinlock_t * lock)
       /*
        * See notes in ptw32_spinlock_check_need_init() above also.
        */
-      EnterCriticalSection (&ptw32_spinlock_test_init_lock);
+      ptw32_mcs_local_node_t node;
+
+      ptw32_mcs_lock_acquire(&ptw32_spinlock_test_init_lock, &node);
 
       /*
        * Check again.
@@ -105,7 +107,7 @@ pthread_spin_destroy (pthread_spinlock_t * lock)
 	  result = EBUSY;
 	}
 
-      LeaveCriticalSection (&ptw32_spinlock_test_init_lock);
+       ptw32_mcs_lock_release(&node);
     }
 
   return (result);

pthread_timechange_handler_np.c

@@ -90,8 +90,9 @@ pthread_timechange_handler_np (void *arg)
 {
   int result = 0;
   pthread_cond_t cv;
+  ptw32_mcs_local_node_t node;
 
-  EnterCriticalSection (&ptw32_cond_list_lock);
+  ptw32_mcs_lock_acquire(&ptw32_cond_list_lock, &node);
 
   cv = ptw32_cond_list_head;
 
@@ -101,7 +102,7 @@ pthread_timechange_handler_np (void *arg)
       cv = cv->next;
     }
 
-  LeaveCriticalSection (&ptw32_cond_list_lock);
+  ptw32_mcs_lock_release(&node);
 
   return (void *) (size_t) (result != 0 ? EAGAIN : 0);
 }

ptw32_cond_check_need_init.c

@@ -43,29 +43,13 @@ INLINE int
 ptw32_cond_check_need_init (pthread_cond_t * cond)
 {
   int result = 0;
+  ptw32_mcs_local_node_t node;
 
   /*
    * The following guarded test is specifically for statically
    * initialised condition variables (via PTHREAD_OBJECT_INITIALIZER).
-   *
-   * Note that by not providing this synchronisation we risk
-   * introducing race conditions into applications which are
-   * correctly written.
-   *
-   * Approach
-   * --------
-   * We know that static condition variables will not be PROCESS_SHARED
-   * so we can serialise access to internal state using
-   * Win32 Critical Sections rather than Win32 Mutexes.
-   *
-   * If using a single global lock slows applications down too much,
-   * multiple global locks could be created and hashed on some random
-   * value associated with each mutex, the pointer perhaps. At a guess,
-   * a good value for the optimal number of global locks might be
-   * the number of processors + 1.
-   *
    */
-  EnterCriticalSection (&ptw32_cond_test_init_lock);
+  ptw32_mcs_lock_acquire(&ptw32_cond_test_init_lock, &node);
 
   /*
    * We got here possibly under race
@@ -88,7 +72,7 @@ ptw32_cond_check_need_init (pthread_cond_t * cond)
       result = EINVAL;
     }
 
-  LeaveCriticalSection (&ptw32_cond_test_init_lock);
+  ptw32_mcs_lock_release(&node);
 
   return result;
 }