2000-08-18 Ross Johnson <rpj@special.ise.canberra.edu.au>

	* mutex.c (pthread_mutex_destroy): Check that the mutex isn't
	held; invalidate the mutex as early as possible to avoid
	contention; not perfect - FIXME!

	* rwlock.c (pthread_rwlock_init): Remove redundant assignment
	to "rw".
	(pthread_rwlock_destroy): Invalidate the rwlock before
	freeing up any of it's resources - to avoid contention.

	* private.c (ptw32_tkAssocCreate): Change assoc->lock
	to use a dynamically initialised mutex - only consumes
	a W32 mutex or critical section when first used,
	not before.

	* mutex.c (pthread_mutex_init): Remove redundant assignment
	to "mx".
	(pthread_mutexattr_destroy): Set attribute to NULL
	before freeing it's memory - to avoid contention.

4 files changed, 81 insertions, 28 deletions

diff --git a/ChangeLog b/ChangeLog
index e2b48ee..7c6b731 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,4 +1,23 @@
-2000-08-18  Ross Johnson  <rpj@setup1.ise.canberra.edu.au>
+2000-08-18  Ross Johnson  <rpj@special.ise.canberra.edu.au>
+
+	* mutex.c (pthread_mutex_destroy): Check that the mutex isn't
+	held; invalidate the mutex as early as possible to avoid
+	contention; not perfect - FIXME!
+
+	* rwlock.c (pthread_rwlock_init): Remove redundant assignment
+	to "rw".
+	(pthread_rwlock_destroy): Invalidate the rwlock before
+	freeing up any of it's resources - to avoid contention.
+
+	* private.c (ptw32_tkAssocCreate): Change assoc->lock
+	to use a dynamically initialised mutex - only consumes
+	a W32 mutex or critical section when first used,
+	not before.
+
+	* mutex.c (pthread_mutex_init): Remove redundant assignment
+	to "mx".
+	(pthread_mutexattr_destroy): Set attribute to NULL
+	before freeing it's memory - to avoid contention.
 
 	* implement.h (PTW32_EPS_CANCEL/PTW32_EPS_EXIT):
 	Must be defined for all compilers - used as generic
diff --git a/mutex.c b/mutex.c
index 3e0e484..42c0844 100644
--- a/mutex.c
+++ b/mutex.c
@@ -96,8 +96,6 @@ pthread_mutex_init(pthread_mutex_t *mutex, const pthread_mutexattr_t *attr)
       return EINVAL;
     }
 
-  mx = *mutex;
-
   mx = (pthread_mutex_t) calloc(1, sizeof(*mx));
 
   if (mx == NULL)
@@ -213,20 +211,40 @@ pthread_mutex_destroy(pthread_mutex_t *mutex)
     {
       mx = *mutex;
 
-      if (mx->mutex == 0)
-	{
-	  DeleteCriticalSection(&mx->cs);
-	}
-      else
-	{
-	  result = (CloseHandle (mx->mutex) ? 0 : EINVAL);
-	}
-
-      if (result == 0)
+      if ((result = pthread_mutex_trylock(&mx)) == 0)
         {
-          mx->mutex = 0;
-          free(mx);
+          /*
+           * FIXME!!!
+           * The mutex isn't held by another thread but we could still
+           * be too late invalidating the mutex below. Yet we can't do it
+           * earlier in case another thread needs to unlock the mutex
+           * that it's holding.
+           */
           *mutex = NULL;
+
+          pthread_mutex_unlock(&mx);
+
+          if (mx->mutex == 0)
+            {
+              DeleteCriticalSection(&mx->cs);
+            }
+          else
+            {
+              result = (CloseHandle (mx->mutex) ? 0 : EINVAL);
+            }
+
+          if (result == 0)
+            {
+              mx->mutex = 0;
+              free(mx);
+            }
+          else
+            {
+              /*
+               * Restore the mutex before we return the error.
+               */
+              *mutex = mx;
+            }
         }
     }
   else
@@ -291,16 +309,17 @@ pthread_mutexattr_init (pthread_mutexattr_t * attr)
       * ------------------------------------------------------
       */
 {
-  pthread_mutexattr_t attr_result;
+  pthread_mutexattr_t ma;
   int result = 0;
 
-  attr_result = (pthread_mutexattr_t) calloc (1, sizeof (*attr_result));
+  ma = (pthread_mutexattr_t) calloc (1, sizeof (*ma));
 
-  result = (attr_result == NULL)
-    ? ENOMEM
-    : 0;
+  if (ma == NULL)
+    {
+      result = ENOMEM;
+    }
 
-  *attr = attr_result;
+  *attr = ma;
 
   return (result);
 
@@ -343,9 +362,11 @@ pthread_mutexattr_destroy (pthread_mutexattr_t * attr)
     }
   else
     {
-      free (*attr);
+      pthread_mutexattr_t ma = *attr;
 
       *attr = NULL;
+      free (ma);
+
       result = 0;
     }
 
diff --git a/private.c b/private.c
index 0f8a175..905a7f6 100644
--- a/private.c
+++ b/private.c
@@ -391,8 +391,7 @@ ptw32_tkAssocCreate (ThreadKeyAssoc ** assocP,
    * Have to create an association and add it
    * to both the key and the thread.
    */
-  assoc = (ThreadKeyAssoc *)
-    calloc (1, sizeof (*assoc));
+  assoc = (ThreadKeyAssoc *) calloc (1, sizeof (*assoc));
 
   if (assoc == NULL)
     {
@@ -400,11 +399,22 @@ ptw32_tkAssocCreate (ThreadKeyAssoc ** assocP,
       goto FAIL0;
     }
 
+#if 0
+
   if ((result = pthread_mutex_init (&(assoc->lock), NULL)) != 0)
     {
       goto FAIL1;
     }
 
+#else
+
+  /*
+   * Initialise only when used for the first time.
+   */
+  assoc->lock = PTHREAD_MUTEX_INITIALIZER;
+
+#endif
+
   assoc->thread = thread;
   assoc->key = key;
 
diff --git a/rwlock.c b/rwlock.c
index d02461c..4353847 100644
--- a/rwlock.c
+++ b/rwlock.c
@@ -94,8 +94,6 @@ pthread_rwlock_init(pthread_rwlock_t *rwlock, const pthread_rwlockattr_t *attr)
         return EINVAL;
       }
 
-    rw = *rwlock;
-
     rw = (pthread_rwlock_t) calloc(1, sizeof(*rw));
 
     if (rw == NULL)
@@ -182,14 +180,19 @@ pthread_rwlock_destroy(pthread_rwlock_t *rwlock)
           }
         else
           {
+            /*
+             * Need to NULL this before we start freeing up
+             * and destroying it's components.
+             */
+            *rwlock = NULL;
             rw->rw_magic = 0;
+
             (void) pthread_mutex_unlock(&(rw->rw_lock));
+
             (void) pthread_cond_destroy(&(rw->rw_condreaders));
             (void) pthread_cond_destroy(&(rw->rw_condwriters));
             (void) pthread_mutex_destroy(&(rw->rw_lock));
- 
             free(rw);
-            *rwlock = NULL;
            }
       }
     else