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

        * condvar.c (ptw32_cond_wait_cleanup): Ensure that all
        waking threads check if they are the last, and notify
        the broadcaster if so - even if an error occurs in the
        waiter.

        * semaphore.c (_decrease_semaphore): Should be
        a call to ptw32_decrease_semaphore.
        (_increase_semaphore): Should be a call to
        ptw32_increase_semaphore.

        * misc.c (ptw32_cancelable_wait): Renamed from
        CancelableWait.
        (cond_wait_cleanup*): Rename to ptw32_cond_wait_cleanup*.
        (ptw32_cond_timedwait): Add comments.

5 files changed, 54 insertions, 34 deletions

diff --git a/ChangeLog b/ChangeLog
index d672b3e..f032072 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,10 +1,24 @@
 2000-09-02  Ross Johnson  <rpj@special.ise.canberra.edu.au>
 
+	* condvar.c (ptw32_cond_wait_cleanup): Ensure that all
+	waking threads check if they are the last, and notify
+	the broadcaster if so - even if an error occurs in the
+	waiter.
+
+	* semaphore.c (_decrease_semaphore): Should be
+	a call to ptw32_decrease_semaphore.
+	(_increase_semaphore): Should be a call to
+	ptw32_increase_semaphore.
+
+	* misc.c (ptw32_cancelable_wait): Renamed from
+	CancelableWait.
 	* rwlock.c (_rwlock_check*): Renamed to
 	ptw32_rwlock_check*.
 	* mutex.c (_mutex_check*): Renamed to ptw32_mutex_check*.
 	* condvar.c (cond_timed*): Renamed to ptw32_cond_timed*.
 	(_cond_check*): Renamed to ptw32_cond_check*.
+	(cond_wait_cleanup*): Rename to ptw32_cond_wait_cleanup*.
+	(ptw32_cond_timedwait): Add comments.
 
 2000-08-22  Ross Johnson  <rpj@setup1.ise.canberra.edu.au>
 
diff --git a/condvar.c b/condvar.c
index 3c5999f..a92fdfb 100644
--- a/condvar.c
+++ b/condvar.c
@@ -530,19 +530,23 @@ typedef struct {
   pthread_mutex_t * mutexPtr;
   pthread_cond_t cv;
   int * resultPtr;
-} cond_wait_cleanup_args_t;
+} ptw32_cond_wait_cleanup_args_t;
 
 static void
-cond_wait_cleanup(void * args)
+ptw32_cond_wait_cleanup(void * args)
 {
-  cond_wait_cleanup_args_t * cleanup_args = (cond_wait_cleanup_args_t *) args;
+  ptw32_cond_wait_cleanup_args_t * cleanup_args = (ptw32_cond_wait_cleanup_args_t *) args;
   pthread_mutex_t * mutexPtr = cleanup_args->mutexPtr;
   pthread_cond_t cv = cleanup_args->cv;
   int * resultPtr = cleanup_args->resultPtr;
-  int lock_result;
   int lastWaiter = FALSE;
 
-  if ((lock_result = pthread_mutex_lock (&(cv->waitersLock))) == 0)
+  /*
+   * Whether we got here legitimately or because of an error we
+   * indicate that we are no longer waiting. The alternative
+   * will result in never signaling the broadcasting thread.
+   */
+  if (pthread_mutex_lock (&(cv->waitersLock)) == 0)
     {
       /*
        * The waiter is responsible for decrementing
@@ -558,40 +562,42 @@ cond_wait_cleanup(void * args)
           cv->wasBroadcast = FALSE;
         }
 
-      lock_result = pthread_mutex_unlock (&(cv->waitersLock));
+      (void) pthread_mutex_unlock (&(cv->waitersLock));
     }
 
-  if ((*resultPtr == 0 || *resultPtr == ETIMEDOUT) && lock_result == 0)
+  /*
+   * If we are the last waiter on this broadcast
+   * let the thread doing the broadcast proceed
+   */
+  if (lastWaiter && !SetEvent (cv->waitersDone))
     {
-      if (lastWaiter)
-        {
-          /*
-           * If we are the last waiter on this broadcast
-           * let the thread doing the broadcast proceed
-           */
-          if (!SetEvent (cv->waitersDone))
-            {
-              *resultPtr = EINVAL;
-            }
-        }
+      *resultPtr = EINVAL;
     }
 
   /*
    * We must always regain the external mutex, even when
    * errors occur, because that's the guarantee that we give
-   * to our callers
+   * to our callers.
+   *
+   * Note that the broadcasting thread may already own the lock.
+   * The standard actually requires that the signaling thread hold
+   * the lock at the time that it signals if the developer wants
+   * predictable scheduling behaviour. It's up to the developer.
+   * In that case all waiting threads will block here until
+   * the broadcasting thread releases the lock, having been
+   * notified by the last waiting thread (SetEvent call above).
    */
   (void) pthread_mutex_lock (mutexPtr);
 }
 
 static int
 ptw32_cond_timedwait (pthread_cond_t * cond, 
-		pthread_mutex_t * mutex,
-		const struct timespec *abstime)
+                      pthread_mutex_t * mutex,
+                      const struct timespec *abstime)
 {
   int result = 0;
   pthread_cond_t cv;
-  cond_wait_cleanup_args_t cleanup_args;
+  ptw32_cond_wait_cleanup_args_t cleanup_args;
 
   if (cond == NULL || *cond == NULL)
     {
@@ -644,15 +650,15 @@ ptw32_cond_timedwait (pthread_cond_t * cond,
   cleanup_args.cv = cv;
   cleanup_args.resultPtr = &result;
 
-  pthread_cleanup_push (cond_wait_cleanup, (void *) &cleanup_args);
+  pthread_cleanup_push (ptw32_cond_wait_cleanup, (void *) &cleanup_args);
 
   if ((result = pthread_mutex_unlock (mutex)) == 0)
     {
       /*
        * Wait to be awakened by
        *              pthread_cond_signal, or
-       *              pthread_cond_broadcast
-       *              timeout
+       *              pthread_cond_broadcast, or
+       *              a timeout
        *
        * Note: 
        *      ptw32_sem_timedwait is a cancelation point,
@@ -668,7 +674,7 @@ ptw32_cond_timedwait (pthread_cond_t * cond,
 	}
     }
 
-  pthread_cleanup_pop (1);
+  pthread_cleanup_pop (1);  /* Always cleanup */
 
   /*
    * "result" can be modified by the cleanup handler.
@@ -737,8 +743,8 @@ pthread_cond_wait (pthread_cond_t * cond,
 
 int
 pthread_cond_timedwait (pthread_cond_t * cond, 
-		pthread_mutex_t * mutex,
-		const struct timespec *abstime)
+                        pthread_mutex_t * mutex,
+                        const struct timespec *abstime)
      /*
       * ------------------------------------------------------
       * DOCPUBLIC
diff --git a/misc.c b/misc.c
index 6228315..724bbc2 100644
--- a/misc.c
+++ b/misc.c
@@ -237,7 +237,7 @@ pthread_equal (pthread_t t1, pthread_t t2)
 
 
 static int
-CancelableWait (HANDLE waitHandle, DWORD timeout)
+ptw32_cancelable_wait (HANDLE waitHandle, DWORD timeout)
      /*
       * -------------------------------------------------------------------
       * This provides an extra hook into the pthread_cancel
@@ -346,13 +346,13 @@ CancelableWait (HANDLE waitHandle, DWORD timeout)
 int
 pthreadCancelableWait (HANDLE waitHandle)
 {
-  return (CancelableWait(waitHandle, INFINITE));
+  return (ptw32_cancelable_wait(waitHandle, INFINITE));
 }
 
 int
 pthreadCancelableTimedWait (HANDLE waitHandle, DWORD timeout)
 {
-  return (CancelableWait(waitHandle, timeout));
+  return (ptw32_cancelable_wait(waitHandle, timeout));
 }
 
 
diff --git a/pthread.h b/pthread.h
index 7f43f43..c25b1fd 100644
--- a/pthread.h
+++ b/pthread.h
@@ -686,7 +686,7 @@ struct ptw32_cleanup_t
 	    {
 	      if ( executeIt && ((void *) cleanUpRout != NULL) )
 		{
-          (void) (*cleanUpRout)( obj );
+                  (void) (*cleanUpRout)( obj );
 		}
 	    }
 
diff --git a/semaphore.c b/semaphore.c
index 366c2de..b388887 100644
--- a/semaphore.c
+++ b/semaphore.c
@@ -382,7 +382,7 @@ sem_wait (sem_t * sem)
 
 #ifdef NEED_SEM
 
-  _decrease_semaphore(sem);
+  ptw32_decrease_semaphore(sem);
 
 #endif /* NEED_SEM */
 
@@ -426,7 +426,7 @@ sem_post (sem_t * sem)
 
 #ifdef NEED_SEM
 
-  else if (! _increase_semaphore (sem, 1))
+  else if (! ptw32_increase_semaphore (sem, 1))
 
 #else /* NEED_SEM */