Changes to mutexes and semaphores - considered alpha for now

25 files changed, 277 insertions, 92 deletions

diff --git a/ChangeLog b/ChangeLog
index ceec5f0..a933330 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,27 @@
+2004-10-22  Ross Johnson  <rpj at callisto.canberra.edu.au>

+
+	* sem_init.c (sem_init): Introduce a 'lock' element in order to
+	replace the interlocked operations with conventional serialisation.
+	This is needed in order to be able to atomically modify the sema value
+	and perform Win32 sema release operations. Win32 semaphores are used
+	instead of events in order to support efficient multiple posting.
+	If the whole modify/release isn't atomic, a race between sem_timedwait()
+	and sem_post() could result in a release when there is no waiting
+	semaphore, which would cause too many threads to proceed.
+	* sem_wait.c (sem_wait): Use new 'lock'element.
+	* sem_timedwait.c (sem_timedwait): Likewise.
+	* sem_trywait.c (sem_trywait): Likewise.
+	* sem_post.c (sem_post): Likewise.
+	* sem_post_multiple.c (sem_post_multiple): Likewise.
+	* sem_getvalue.c (sem_getvalue): Likewise.
+	* ptw32_semwait.c (ptw32_semwait): Likewise.
+	* sem_destroy.c (sem_destroy): Likewise; also tightened the conditions
+	for semaphore destruction; in particular, a semaphore will not be
+	destroyed if it has waiters.
+	* sem_timedwait.c (sem_timedwait): Added cancel cleanup handler to
+	restore sema value when cancelled.
+	* sem_wait.c (sem_wait): Likewise.
+
 2004-10-21  Ross Johnson  <rpj at callisto.canberra.edu.au>

 
 	* pthread_mutex_unlock.c (pthread_mutex_unlock): Must use PulseEvent()
diff --git a/implement.h b/implement.h
index 9cb3cb3..1f6179e 100644
--- a/implement.h
+++ b/implement.h
@@ -173,6 +173,7 @@ struct sem_t_
   HANDLE event;
 #else				/* NEED_SEM */
   int value;
+  pthread_mutex_t lock;
   HANDLE sem;
 #endif				/* NEED_SEM */
 };
diff --git a/pthread_cond_signal.c b/pthread_cond_signal.c
index 84ace67..fb1c81e 100644
--- a/pthread_cond_signal.c
+++ b/pthread_cond_signal.c
@@ -165,7 +165,6 @@
 #include "pthread.h"
 #include "implement.h"
 
-
 static INLINE int
 ptw32_cond_unblock (pthread_cond_t * cond, int unblockAll)
      /*
diff --git a/pthread_mutex_unlock.c b/pthread_mutex_unlock.c
index 9b8e504..9002df5 100644
--- a/pthread_mutex_unlock.c
+++ b/pthread_mutex_unlock.c
@@ -100,7 +100,7 @@ pthread_mutex_unlock (pthread_mutex_t * mutex)
 							 (LONG) 0) < 0)
 		    {
 		      /* Someone may be waiting on that mutex */
-		      if (SetEvent (mx->event) == 0)
+		      if (PulseEvent (mx->event) == 0)
 			{
 			  result = EINVAL;
 			}
diff --git a/ptw32_semwait.c b/ptw32_semwait.c
index 2e9c883..e8b7ace 100644
--- a/ptw32_semwait.c
+++ b/ptw32_semwait.c
@@ -67,10 +67,10 @@ ptw32_semwait (sem_t * sem)
       */
 {
   int result = 0;
-
   DWORD status;
+  sem_t s = *sem;
 
-  if (sem == NULL)
+  if (s == NULL)
     {
       result = EINVAL;
     }
@@ -79,19 +79,30 @@ ptw32_semwait (sem_t * sem)
 
 #ifdef NEED_SEM
 
-      status = WaitForSingleObject ((*sem)->event, INFINITE);
+      status = WaitForSingleObject (s->event, INFINITE);
 
 #else /* NEED_SEM */
 
-      if (InterlockedDecrement((LPLONG) &(*sem)->value) < 0)
+      if ((result = pthread_mutex_lock (&s->lock)) == 0)
         {
-          /* Must wait */
-          status = WaitForSingleObject ((*sem)->sem, INFINITE);
+          int v = --s->value;
+
+          (void) pthread_mutex_unlock (&s->lock);
+
+          if (v < 0)
+            {
+              /* Must wait */
+              status = WaitForSingleObject (s->sem, INFINITE);
+            }
+          else
+	    {
+	      return 0;
+	    }
         }
       else
-	{
-	  return 0;
-	}
+        {
+          return result;
+        }
 
 #endif
 
@@ -109,7 +120,7 @@ ptw32_semwait (sem_t * sem)
       else
 	{
 #ifndef NEED_SEM
-	  (void) InterlockedIncrement((LPLONG) &(*sem)->value);
+	  (void) InterlockedIncrement((LPLONG) &s->value);
 #endif
 	  result = EINVAL;
 	}
diff --git a/sem_destroy.c b/sem_destroy.c
index 733afd3..3463981 100644
--- a/sem_destroy.c
+++ b/sem_destroy.c
@@ -98,11 +98,44 @@ sem_destroy (sem_t * sem)
 
 #else /* NEED_SEM */
 
-      if (!CloseHandle (s->sem))
-	{
-	  *sem = s;
-	  result = EINVAL;
-	}
+      if ((result = pthread_mutex_trylock (&s->lock)) == 0)
+        {
+          if (s->value >= 0)
+            {
+              (void) pthread_mutex_unlock (&s->lock);
+
+              if (!CloseHandle (s->sem))
+	        {
+	          *sem = s;
+	          result = EINVAL;
+	        }
+              else if ((result = pthread_mutex_destroy (&s->lock)) != 0)
+                {
+                  s->sem = CreateSemaphore (NULL,      /* Always NULL */
+                                            (long) 0,  /* Force threads to wait */
+                                            (long) _POSIX_SEM_VALUE_MAX,       /* Maximum value */
+                                            NULL);     /* Name */
+                  if (s->sem == 0)
+                    {
+                      /* We just have to pretend that we've destroyed the semaphore
+                       * even though we're leaving a mutex around.
+                       */
+                      result = 0;
+                    }
+                  else
+                    {
+                      *sem = s;
+                      if (result != EBUSY)
+                        result = EINVAL;
+                    }
+                }
+            }
+          else
+            {
+              (void) pthread_mutex_unlock (&s->lock);
+              result = EBUSY;
+            }
+        }
 
 #endif /* NEED_SEM */
 
diff --git a/sem_getvalue.c b/sem_getvalue.c
index eb6e6af..f2efd10 100644
--- a/sem_getvalue.c
+++ b/sem_getvalue.c
@@ -52,9 +52,7 @@ sem_getvalue (sem_t * sem, int *sval)
       * ------------------------------------------------------
       * DOCPUBLIC
       *      This function stores the current count value of the
-      *      semaphore. If the count is negative, it's absolute
-      *      value is the number of threads currently waiting on
-      *      the semaphore.
+      *      semaphore.
       * RESULTS
       *
       * Return value
@@ -88,21 +86,27 @@ sem_getvalue (sem_t * sem, int *sval)
     {
       long value;
       register sem_t s = *sem;
+      int result = 0;
 
 #ifdef NEED_SEM
 
       EnterCriticalSection (&s->sem_lock_cs);
       value = s->value;
       LeaveCriticalSection (&s->sem_lock_cs);
+      *sval = value;
 
 #else
 
-      value = s->value;
+      if ((result = pthread_mutex_lock(&s->lock)) == 0)
+        {
+          value = s->value;
+          (void) pthread_mutex_unlock(&s->lock);
+          *sval = value;
+        }
 
 #endif
 
-      *sval = value;
-      return 0;
+      return result;
     }
 
 }				/* sem_getvalue */
diff --git a/sem_init.c b/sem_init.c
index af0e285..fb596ae 100644
--- a/sem_init.c
+++ b/sem_init.c
@@ -128,17 +128,27 @@ sem_init (sem_t * sem, int pshared, unsigned int value)
 #else /* NEED_SEM */
 
 	  s->value = value;
-	  s->sem = CreateSemaphore (NULL,	/* Always NULL */
-				    (long) 0,	/* Force threads to wait */
-				    (long) _POSIX_SEM_VALUE_MAX,	/* Maximum value */
-				    NULL);	/* Name */
-
-	  if (0 == s->sem)
+	  if (pthread_mutex_init(&s->lock, NULL) == 0)
+	    {
+	      if ((s->sem = CreateSemaphore (NULL,	/* Always NULL */
+					     (long) 0,	/* Force threads to wait */
+					     (long) _POSIX_SEM_VALUE_MAX,	/* Maximum value */
+					     NULL)) == 0)	/* Name */
+		{
+		  (void) pthread_mutex_destroy(&s->lock);
+		  result = ENOSPC;
+		}
+	    }
+	  else
 	    {
-	      free (s);
 	      result = ENOSPC;
 	    }
 
+	  if (result != 0)
+	    {
+	      free(s);
+	    }
+
 #endif /* NEED_SEM */
 
 	}
diff --git a/sem_post.c b/sem_post.c
index 7e20659..5b678c6 100644
--- a/sem_post.c
+++ b/sem_post.c
@@ -73,8 +73,11 @@ sem_post (sem_t * sem)
       */
 {
   int result = 0;
+#ifndef NEED_SEM
+  sem_t s = *sem;
+#endif
 
-  if (sem == NULL || *sem == NULL)
+  if (s == NULL)
     {
       result = EINVAL;
     }
@@ -82,18 +85,25 @@ sem_post (sem_t * sem)
 #ifdef NEED_SEM
 
   else if (!ptw32_increase_semaphore (sem, 1))
+    {
+      result = EINVAL;
+    }
 
 #else /* NEED_SEM */
 
-    else if (InterlockedIncrement((LPLONG) &(*sem)->value) <= 0
-	     && !ReleaseSemaphore((*sem)->sem, 1, NULL))
-
-#endif /* NEED_SEM */
-
+  else if ((result = pthread_mutex_lock (&s->lock)) == 0)
     {
-      result = EINVAL;
+      if (++s->value <= 0
+	  && !ReleaseSemaphore (s->sem, 1, NULL))
+	{
+	  result = EINVAL;
+	}
+      
+      (void) pthread_mutex_unlock (&s->lock);
     }
 
+#endif /* NEED_SEM */
+
   if (result != 0)
     {
       errno = result;
diff --git a/sem_post_multiple.c b/sem_post_multiple.c
index e30e01a..89368f0 100644
--- a/sem_post_multiple.c
+++ b/sem_post_multiple.c
@@ -78,9 +78,10 @@ sem_post_multiple (sem_t * sem, int count)
   int result = 0;
 #ifndef NEED_SEM
   long waiters;
+  sem_t s = *sem;
 #endif
 
-  if (sem == NULL || *sem == NULL || count <= 0)
+  if (s == NULL || count <= 0)
     {
       result = EINVAL;
     }
@@ -88,18 +89,28 @@ sem_post_multiple (sem_t * sem, int count)
 #ifdef NEED_SEM
 
   else if (!ptw32_increase_semaphore (sem, count))
+    {
+      result = EINVAL;
+    }
 
 #else /* NEED_SEM */
 
-    else if ((waiters = -InterlockedExchangeAdd((LPLONG) &(*sem)->value, (LONG) count)) > 0
-	     && !ReleaseSemaphore((*sem)->sem,  (waiters<=count)?waiters:count, 0))
-
-#endif /* NEED_SEM */
-
+  else if ((result = pthread_mutex_lock (&s->lock)) == 0)
     {
-      result = EINVAL;
+      waiters = -s->value;
+      s->value += count;
+      if (waiters > 0)
+        {
+          if (!ReleaseSemaphore (s->sem,  (waiters<=count)?waiters:count, 0))
+            {
+              result = EINVAL;
+            }
+        }
+      (void) pthread_mutex_unlock (&s->lock);
     }
 
+#endif /* NEED_SEM */
+
   if (result != 0)
     {
       errno = result;
diff --git a/sem_timedwait.c b/sem_timedwait.c
index e7fa55c..7282a97 100644
--- a/sem_timedwait.c
+++ b/sem_timedwait.c
@@ -52,6 +52,17 @@
 #include "semaphore.h"
 #include "implement.h"
 
+static void
+ptw32_sem_timedwait_cleanup (void * sem)
+{
+  sem_t s = (sem_t) sem;
+
+  if (pthread_mutex_lock (&s->lock) == 0)
+    {
+      s->value++;
+      (void) pthread_mutex_unlock (&s->lock);
+    }
+}
 
 int
 sem_timedwait (sem_t * sem, const struct timespec *abstime)
@@ -94,6 +105,7 @@ sem_timedwait (sem_t * sem, const struct timespec *abstime)
       */
 {
   int result = 0;
+  sem_t s = *sem;
 
 #ifdef NEED_FTIME
 
@@ -185,19 +197,29 @@ sem_timedwait (sem_t * sem, const struct timespec *abstime)
 
 #ifdef NEED_SEM
 
-      result = (pthreadCancelableTimedWait ((*sem)->event, milliseconds));
+      result = (pthreadCancelableTimedWait (s->event, milliseconds));
 
 #else /* NEED_SEM */
 
-      if (InterlockedDecrement((LPLONG) &(*sem)->value) < 0)
-        {
-          /* Must wait */
-	  result = pthreadCancelableTimedWait ((*sem)->sem, milliseconds);
-	  if (result != 0)
+      if ((result = pthread_mutex_lock (&s->lock)) == 0)
+	{
+	  int v = --s->value;
+	  (void) pthread_mutex_unlock (&s->lock);
+
+	  if (v < 0)
 	    {
-              (void) InterlockedIncrement((LPLONG) &(*sem)->value);
+	      /* Must wait */
+              pthread_cleanup_push(ptw32_sem_timedwait_cleanup, s);
+	      result = pthreadCancelableTimedWait (s->sem, milliseconds);
+              pthread_cleanup_pop(0);
+	      if (result != 0
+		  && pthread_mutex_lock (&s->lock) == 0)
+		{
+		  s->value++;
+		  (void) pthread_mutex_unlock (&s->lock);
+		}
 	    }
-        }
+	}
 
 #endif
 
diff --git a/sem_trywait.c b/sem_trywait.c
index 253c10e..fbc457c 100644
--- a/sem_trywait.c
+++ b/sem_trywait.c
@@ -87,15 +87,21 @@ sem_trywait (sem_t * sem)
 #else /* NEED_SEM */
 
   int result = 0;
+  sem_t s = *sem;
 
-  if (sem == NULL || *sem == NULL)
+  if (s == NULL)
     {
       result = EINVAL;
     }
-  else if (InterlockedDecrement((LPLONG) &(*sem)->value) < 0)
+  else if ((result = pthread_mutex_lock (&s->lock)) == 0)
     {
-      (void) InterlockedIncrement((LPLONG) &(*sem)->value);
-      result = EAGAIN;
+      if (--s->value < 0)
+	{
+	  s->value++;
+	  result = EAGAIN;
+	}
+
+      (void) pthread_mutex_unlock (&s->lock);
     }
 
   if (result != 0)
diff --git a/sem_wait.c b/sem_wait.c
index 0c2ed59..5142172 100644
--- a/sem_wait.c
+++ b/sem_wait.c
@@ -45,6 +45,17 @@
 #include "semaphore.h"
 #include "implement.h"
 
+static void
+ptw32_sem_wait_cleanup(void * sem)
+{
+  sem_t s = (sem_t) sem;
+
+  if (pthread_mutex_lock (&s->lock) == 0)
+    {
+      s->value++;
+      (void) pthread_mutex_unlock (&s->lock);
+    }
+}
 
 int
 sem_wait (sem_t * sem)
@@ -78,8 +89,9 @@ sem_wait (sem_t * sem)
       */
 {
   int result = 0;
+  sem_t s = *sem;
 
-  if (sem == NULL || *sem == NULL)
+  if (s == NULL)
     {
       result = EINVAL;
     }
@@ -88,16 +100,23 @@ sem_wait (sem_t * sem)
 
 #ifdef NEED_SEM
 
-      result = pthreadCancelableWait ((*sem)->event);
+      result = pthreadCancelableWait (s->event);
 
 #else /* NEED_SEM */
 
-      sem_t s = *sem;
-
-      if (InterlockedDecrement((LPLONG) &s->value) < 0)
+      if ((result = pthread_mutex_lock (&s->lock)) == 0)
 	{
-	  /* Must wait */
-	  result = pthreadCancelableWait (s->sem);
+	  int v = --s->value;
+
+	  (void) pthread_mutex_unlock (&s->lock);
+
+	  if (v < 0)
+	    {
+	      /* Must wait */
+	      pthread_cleanup_push(ptw32_sem_wait_cleanup, s);
+	      result = pthreadCancelableWait (s->sem);
+	      pthread_cleanup_pop(0);
+	    }
 	}
      
 #endif /* NEED_SEM */
diff --git a/tests/ChangeLog b/tests/ChangeLog
index 65d52f1..27f4eea 100644
--- a/tests/ChangeLog
+++ b/tests/ChangeLog
@@ -1,3 +1,20 @@
+2004-10-23  Ross Johnson  <rpj@callisto.canberra.edu.au>

+
+	* condvar3.c: Fixed mutex operations that were incorrectly
+	placed in relation to their condition variable operations.
+	The error became evident after sem_destroy() was rewritten
+	and conditions for destroing the semaphore were tightened.
+	As a result, pthread_cond_destroy() was not able to
+	destroy the cv queueing sempahore.
+	* condvar3_1.c: Likewise.
+	* condvar3_2.c: Likewise.
+	* condvar4.c: Likewise.
+	* condvar5.c: Likewise.
+	* condvar6.c: Likewise.
+	* condvar7.c: Likewise.
+	* condvar8.c: Likewise.
+	* condvar9.c: Likewise.
+
 2004-10-19  Ross Johnson  <rpj@callisto.canberra.edu.au>

 
 	* semaphore3.c: New test.
diff --git a/tests/condvar1_1.c b/tests/condvar1_1.c
index 0090fc0..17f4a31 100644
--- a/tests/condvar1_1.c
+++ b/tests/condvar1_1.c
@@ -38,7 +38,7 @@
  *
  * Test Method (Validation or Falsification):
  * - Validation:
- *   Initiate and destry several CVs in random order.
+ *   Initiate and destroy several CVs in random order.
  *
  * Requirements Tested:
  * - 
diff --git a/tests/condvar3.c b/tests/condvar3.c
index aeeb915..8fa9ef7 100644
--- a/tests/condvar3.c
+++ b/tests/condvar3.c
@@ -97,25 +97,29 @@ mythread(void * arg)
 
   shared++;
 
-  assert(pthread_mutex_unlock(&mutex) == 0);
-
   if ((result = pthread_cond_signal(&cv)) != 0)
     {
       printf("Error = %s\n", error_string[result]);
     }
   assert(result == 0);
 
+  assert(pthread_mutex_unlock(&mutex) == 0);
+
   return (void *) 0;
 }
 
 int
 main()
 {
+  pthread_attr_t attr;
   pthread_t t[NUMTHREADS];
   struct timespec abstime = { 0, 0 };
   struct _timeb currSysTime;
   const DWORD NANOSEC_PER_MILLISEC = 1000000;
 
+  assert(pthread_attr_init(&attr) == 0);
+  assert(pthread_attr_setdetachstate(&attr, PTHREAD_CREATE_DETACHED) == 0);
+
   assert((t[0] = pthread_self()) != NULL);
 
   assert(pthread_cond_init(&cv, NULL) == 0);
@@ -130,7 +134,7 @@ main()
   abstime.tv_sec = currSysTime.time;
   abstime.tv_nsec = NANOSEC_PER_MILLISEC * currSysTime.millitm;
 
-  assert(pthread_create(&t[1], NULL, mythread, (void *) 1) == 0);
+  assert(pthread_create(&t[1], &attr, mythread, (void *) 1) == 0);
 
   abstime.tv_sec += 5;
 
diff --git a/tests/condvar3_1.c b/tests/condvar3_1.c
index 200aaf9..c2c3d98 100644
--- a/tests/condvar3_1.c
+++ b/tests/condvar3_1.c
@@ -83,6 +83,7 @@
 static pthread_cond_t cv;
 static pthread_cond_t cv1;
 static pthread_mutex_t mutex;
+static pthread_mutex_t mutex1;
 static struct timespec abstime = { 0, 0 };
 static int timedout = 0;
 static int signaled = 0;
@@ -98,11 +99,12 @@ mythread(void * arg)
 {
   int result;
 
-  assert(pthread_mutex_lock(&mutex) == 0);
-
-  if ( ++waiting == NUMTHREADS)
-    assert(pthread_cond_signal(&cv1) == 0);
+  assert(pthread_mutex_lock(&mutex1) == 0);
+  ++waiting;
+  assert(pthread_cond_signal(&cv1) == 0);
+  assert(pthread_mutex_unlock(&mutex1) == 0);
 
+  assert(pthread_mutex_lock(&mutex) == 0);
   result = pthread_cond_timedwait(&cv, &mutex, &abstime);
   if (result == ETIMEDOUT)
     {
@@ -112,7 +114,6 @@ mythread(void * arg)
     {
       awoken++;
     }
-
   assert(pthread_mutex_unlock(&mutex) == 0);
 
   return arg;
@@ -133,6 +134,7 @@ main()
   assert(pthread_cond_init(&cv1, NULL) == 0);
 
   assert(pthread_mutex_init(&mutex, NULL) == 0);
+  assert(pthread_mutex_init(&mutex1, NULL) == 0);
 
   /* get current system time */
   _ftime(&currSysTime);
@@ -142,7 +144,7 @@ main()
 
   abstime.tv_sec += 5;
 
-  assert(pthread_mutex_lock(&mutex) == 0);
+  assert(pthread_mutex_lock(&mutex1) == 0);
 
   for (i = 1; i <= NUMTHREADS; i++)
     {
@@ -150,19 +152,20 @@ main()
     }
 
   do {
-    assert(pthread_cond_wait(&cv1,&mutex) == 0);
-  } while ( NUMTHREADS != waiting );
+    assert(pthread_cond_wait(&cv1,&mutex1) == 0);
+  } while ( NUMTHREADS > waiting );
 
-  assert(pthread_mutex_unlock(&mutex) == 0);
+  assert(pthread_mutex_unlock(&mutex1) == 0);
 
   for (i = NUMTHREADS/3; i <= 2*NUMTHREADS/3; i++)
     {
+      assert(pthread_mutex_lock(&mutex) == 0);
       assert(pthread_cond_signal(&cv) == 0);
+      assert(pthread_mutex_unlock(&mutex) == 0);
+
       signaled++;
     }
 
-  assert(pthread_cond_destroy(&cv1) == 0);
-
   for (i = 1; i <= NUMTHREADS; i++)
     {
       assert(pthread_join(t[i], (void **) &result) == 0);
@@ -176,6 +179,8 @@ main()
   assert(signaled == awoken);
   assert(timedout == NUMTHREADS - signaled);
 
+  assert(pthread_cond_destroy(&cv1) == 0);
+
   {
   int result = pthread_cond_destroy(&cv);
   if (result != 0)
@@ -189,5 +194,8 @@ main()
   assert(result == 0);
   }
 
+  assert(pthread_mutex_destroy(&mutex1) == 0);
+  assert(pthread_mutex_destroy(&mutex) == 0);
+
   return 0;
 }
diff --git a/tests/condvar3_2.c b/tests/condvar3_2.c
index 865ca9d..525ddf6 100644
--- a/tests/condvar3_2.c
+++ b/tests/condvar3_2.c
@@ -160,10 +160,16 @@ main()
        * and while some are still waking up after timeout.
        * Also tests that redundant broadcasts don't return errors.
        */
+
+      assert(pthread_mutex_lock(&mutex) == 0);
+
       if (awoken > NUMTHREADS/3)
         {
           assert(pthread_cond_broadcast(&cv) == 0);
         }
+
+      assert(pthread_mutex_unlock(&mutex) == 0);
+
     }
 
   assert(awoken == NUMTHREADS - timedout);
@@ -181,5 +187,7 @@ main()
   assert(result == 0);
   }
 
+  assert(pthread_mutex_destroy(&mutex) == 0);
+
   return 0;
 }
diff --git a/tests/condvar4.c b/tests/condvar4.c
index a4f3299..15a32ba 100644
--- a/tests/condvar4.c
+++ b/tests/condvar4.c
@@ -102,10 +102,10 @@ mythread(void * arg)
 
   cvthing.shared++;
 
-  assert(pthread_mutex_unlock(&cvthing.lock) == 0);
-
   assert(pthread_cond_signal(&cvthing.notbusy) == 0);
 
+  assert(pthread_mutex_unlock(&cvthing.lock) == 0);
+
   return (void *) 0;
 }
 
diff --git a/tests/condvar5.c b/tests/condvar5.c
index 53311b4..ff5ad70 100644
--- a/tests/condvar5.c
+++ b/tests/condvar5.c
@@ -101,10 +101,10 @@ mythread(void * arg)
 
   cvthing.shared++;
 
-  assert(pthread_mutex_unlock(&cvthing.lock) == 0);
-
   assert(pthread_cond_broadcast(&cvthing.notbusy) == 0);
 
+  assert(pthread_mutex_unlock(&cvthing.lock) == 0);
+
   return (void *) 0;
 }
 
diff --git a/tests/condvar6.c b/tests/condvar6.c
index 9587781..d642f9c 100644
--- a/tests/condvar6.c
+++ b/tests/condvar6.c
@@ -190,10 +190,10 @@ main()
 
   cvthing.shared++;
 
-  assert(pthread_mutex_unlock(&cvthing.lock) == 0);
-
   assert(pthread_cond_broadcast(&cvthing.notbusy) == 0);
 
+  assert(pthread_mutex_unlock(&cvthing.lock) == 0);
+
   /*
    * Give threads time to complete.
    */
diff --git a/tests/condvar7.c b/tests/condvar7.c
index e79e0f8..a5bf4bf 100644
--- a/tests/condvar7.c
+++ b/tests/condvar7.c
@@ -200,8 +200,6 @@ main()
 
   cvthing.shared++;
 
-  assert(pthread_mutex_unlock(&cvthing.lock) == 0);
-
   /*
    * Cancel one of the threads.
    */
@@ -213,6 +211,8 @@ main()
    */
   assert(pthread_cond_broadcast(&cvthing.notbusy) == 0);
 
+  assert(pthread_mutex_unlock(&cvthing.lock) == 0);
+
   /*
    * Give threads time to complete.
    */
diff --git a/tests/condvar8.c b/tests/condvar8.c
index 4d28f6a..2ab67b3 100644
--- a/tests/condvar8.c
+++ b/tests/condvar8.c
@@ -205,10 +205,10 @@ main()
 
       cvthing.shared++;
 
-      assert(pthread_mutex_unlock(&cvthing.lock) == 0);
-
       assert(pthread_cond_broadcast(&cvthing.notbusy) == 0);
 
+      assert(pthread_mutex_unlock(&cvthing.lock) == 0);
+
       /*
        * Give threads time to complete.
        */
diff --git a/tests/condvar9.c b/tests/condvar9.c
index cf31240..b24d71e 100644
--- a/tests/condvar9.c
+++ b/tests/condvar9.c
@@ -211,13 +211,13 @@ main()
 
       cvthing.shared++;
 
-      assert(pthread_mutex_unlock(&cvthing.lock) == 0);
-
       assert(pthread_cancel(t[(first + last) / 2]) == 0);
       canceledThreads++;
 
       assert(pthread_cond_broadcast(&cvthing.notbusy) == 0);
 
+      assert(pthread_mutex_unlock(&cvthing.lock) == 0);
+
       /*
        * Give threads time to complete.
        */
diff --git a/tests/kill1.c b/tests/kill1.c
index b9477fc..1ffd07a 100644
--- a/tests/kill1.c
+++ b/tests/kill1.c
@@ -77,8 +77,6 @@
 int
 main()
 {
-  void * result = NULL;
-
   assert(pthread_kill(pthread_self(), 1) == EINVAL);
 
   return 0;