''v-2-8-0-release

19 files changed, 191 insertions, 53 deletions

diff --git a/ANNOUNCE b/ANNOUNCE
index 7c37f32..97d94e7 100644
--- a/ANNOUNCE
+++ b/ANNOUNCE
@@ -1,4 +1,4 @@
-	     PTHREADS-WIN32 RELEASE 2.7.0 (2005-06-04)
+	     PTHREADS-WIN32 RELEASE 2.8.0 (2006-12-22)
 	     -----------------------------------------
 	 Web Site: http://sources.redhat.com/pthreads-win32/
 	FTP Site: ftp://sources.redhat.com/pub/pthreads-win32
diff --git a/BUGS b/BUGS
index bed6faf..29cee00 100644
--- a/BUGS
+++ b/BUGS
@@ -126,3 +126,8 @@ implement all the same safeguards as condition variables, making them much
 
 The workaround is to ensure that no more than "count" threads attempt to wait
 at the barrier.
+
+5. Canceling a thread blocked on pthread_once appears not to work in the MSVC++
+version of the library "pthreadVCE.dll". The test case "once3.c" hangs. I have no
+clues on this at present. All other versions pass this test ok - pthreadsVC.dll,
+pthreadsVSE.dll, pthreadsGC.dll and pthreadsGCE.dll. 
diff --git a/ChangeLog b/ChangeLog
index 393cfe6..b85d9f9 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,24 @@
+2006-12-20  Ross Johnson <ross.johnson@homemail.com.au>
+
+        * sem_destroy.c: Fix the race involving invalidation of the sema;
+        fix incorrect return of EBUSY resulting from the mutex trylock
+        on the private mutex guard.
+        * sem_wait.c: Add check for invalid sem_t after acquiring the
+        sem_t state guard mutex and before affecting changes to sema state.
+        * sem_trywait.c: Likewise.
+        * sem_timedwait.c: Likewise.
+        * sem_getvalue.c: Likewise.
+        * sem_post.c: Similar.
+        * sem_post_multiple.c: Likewise.
+        * sem_init.c: Set max Win32 semaphore count to SEM_VALUE_MAX (was
+        _POSIX_SEM_VALUE_MAX, which is a lower value - the minimum).
+
+	* pthread_win32_attach_detach_np.c (pthread_win32_process_attach_np):
+	Load COREDLL.DLL under WINCE to check existence of
+	InterlockedCompareExchange() routine. This used to be done to test
+	for TryEnterCriticalSection() but was removed when this was no
+	longer needed.
+
 2006-01-25  Prashant Thakre <prashant.thakre at gmail.com>
 
 	* pthread_cancel.c: Added _M_IA64 register context support.
diff --git a/NEWS b/NEWS
index 6dd8caf..18fe543 100644
--- a/NEWS
+++ b/NEWS
@@ -1,3 +1,28 @@
+RELEASE 2.8.0
+-------------
+(2006-12-22)
+
+General
+-------
+New bug fixes in this release since 2.7.0 have not been applied to the
+version 1.x.x series. It is probably time to drop version 1.
+
+Testing and verification
+------------------------
+This release has not yet been tested on SMP architechtures. All tests pass
+on a uni-processor system.
+
+Bug fixes
+---------
+Sem_destroy could return EBUSY even though no threads were waiting on the 
+semaphore. Other races around invalidating semaphore structs (internally)
+have been removed as well.
+
+New tests
+---------
+semaphore5.c - tests the bug fix referred to above.
+
+
 RELEASE 2.7.0
 -------------
 (2005-06-04)
diff --git a/pthread_win32_attach_detach_np.c b/pthread_win32_attach_detach_np.c
index 2b99d53..2f0bc37 100644
--- a/pthread_win32_attach_detach_np.c
+++ b/pthread_win32_attach_detach_np.c
@@ -91,11 +91,22 @@ pthread_win32_process_attach_np ()
 
 #endif
 
+#ifdef WINCE
+
+  /*
+   * Load COREDLL and try to get address of InterlockedCompareExchange
+   */
+  ptw32_h_kernel32 = LoadLibrary (TEXT ("COREDLL.DLL"));
+
+#else
+
   /*
    * Load KERNEL32 and try to get address of InterlockedCompareExchange
    */
   ptw32_h_kernel32 = LoadLibrary (TEXT ("KERNEL32.DLL"));
 
+#endif
+
   ptw32_interlocked_compare_exchange =
     (PTW32_INTERLOCKED_LONG (WINAPI *)
      (PTW32_INTERLOCKED_LPLONG, PTW32_INTERLOCKED_LONG,
diff --git a/ptw32_MCS_lock.c b/ptw32_MCS_lock.c
index 478a059..1a143ea 100644
--- a/ptw32_MCS_lock.c
+++ b/ptw32_MCS_lock.c
@@ -50,8 +50,8 @@
  * operations required to add or remove nodes from the queue do not trigger
  * cache-coherence updates.
  *
- * Like 'named' mutexes, MCS locks consume system recourses transiently -
- * they are able to acquire and free recourses automatically - but MCS
+ * Like 'named' mutexes, MCS locks consume system resources transiently -
+ * they are able to acquire and free resources automatically - but MCS
  * locks do not require any unique 'name' to identify the lock to all
  * threads using it.
  *
diff --git a/sem_destroy.c b/sem_destroy.c
index 96734d1..6c98e80 100644
--- a/sem_destroy.c
+++ b/sem_destroy.c
@@ -82,54 +82,52 @@ sem_destroy (sem_t * sem)
   else
     {
       s = *sem;
-      *sem = NULL;
 
-      if ((result = pthread_mutex_trylock (&s->lock)) == 0)
+      if ((result = pthread_mutex_lock (&s->lock)) == 0)
         {
-          if (s->value >= 0)
+          if (s->value < 0)
             {
               (void) pthread_mutex_unlock (&s->lock);
+              result = EBUSY;
+            }
+          else
+            {
+              /* There are no threads currently blocked on this semaphore. */
 
               if (!CloseHandle (s->sem))
 	        {
-	          *sem = s;
+                  (void) pthread_mutex_unlock (&s->lock);
 	          result = EINVAL;
 	        }
-              else if ((result = pthread_mutex_destroy (&s->lock)) != 0)
-                {
+	      else
+	        {
+                  /*
+                   * Invalidate the semaphore handle when we have the lock.
+                   * Other sema operations should test this after acquiring the lock
+                   * to check that the sema is still valid, i.e. before performing any
+                   * operations. This may only be necessary before the sema op routine
+                   * returns so that the routine can return EINVAL - e.g. if setting
+                   * s->value to SEM_VALUE_MAX below does force a fall-through.
+                   */
+                  *sem = NULL;
+
+                  /* Prevent anyone else actually waiting on or posting this sema.
+                   */
+                  s->value = SEM_VALUE_MAX;
 
-#ifdef NEED_SEM
-		  s->sem = CreateEvent (NULL,
-					PTW32_FALSE,    /* manual reset is false */
-					PTW32_FALSE,    /* initial state is unset */
-					NULL);
-#else
-                  s->sem = CreateSemaphore (NULL,      /* Always NULL */
-                                            (long) 0,  /* Force threads to wait */
-                                            (long) _POSIX_SEM_VALUE_MAX,       /* Maximum value */
-                                            NULL);     /* Name */
-#endif
+                  (void) pthread_mutex_unlock (&s->lock);
 
-                  if (s->sem == 0)
+                  do
                     {
-                      /* We just have to pretend that we've destroyed the semaphore
-                       * even though we're leaving a mutex around.
+                      /* Give other threads a chance to run and exit any sema op
+                       * routines. Due to the SEM_VALUE_MAX value, if sem_post or
+                       * sem_wait were blocked by us they should fall through.
                        */
-                      result = 0;
-                    }
-                  else
-                    {
-                      *sem = s;
-                      if (result != EBUSY)
-                        result = EINVAL;
+                      Sleep(0);
                     }
+                  while (pthread_mutex_destroy (&s->lock) == EBUSY);
                 }
             }
-          else
-            {
-              (void) pthread_mutex_unlock (&s->lock);
-              result = EBUSY;
-            }
         }
     }
 
diff --git a/sem_getvalue.c b/sem_getvalue.c
index ae13ef9..baafb02 100644
--- a/sem_getvalue.c
+++ b/sem_getvalue.c
@@ -90,6 +90,15 @@ sem_getvalue (sem_t * sem, int *sval)
 
       if ((result = pthread_mutex_lock(&s->lock)) == 0)
         {
+	  /* See sem_destroy.c
+	   */
+	  if (*sem == NULL)
+	    {
+	      (void) pthread_mutex_unlock (&s->lock);
+	      errno = EINVAL;
+	      return -1;
+	    }
+
           value = s->value;
           (void) pthread_mutex_unlock(&s->lock);
           *sval = value;
diff --git a/sem_init.c b/sem_init.c
index 27d8cbb..02acd90 100644
--- a/sem_init.c
+++ b/sem_init.c
@@ -134,7 +134,7 @@ sem_init (sem_t * sem, int pshared, unsigned int value)
 
 	      if ((s->sem = CreateSemaphore (NULL,	/* Always NULL */
 					     (long) 0,	/* Force threads to wait */
-					     (long) _POSIX_SEM_VALUE_MAX,	/* Maximum value */
+					     (long) SEM_VALUE_MAX,	/* Maximum value */
 					     NULL)) == 0)	/* Name */
 		{
 		  (void) pthread_mutex_destroy(&s->lock);
diff --git a/sem_post.c b/sem_post.c
index 91f9755..c7a7a3c 100644
--- a/sem_post.c
+++ b/sem_post.c
@@ -82,6 +82,15 @@ sem_post (sem_t * sem)
     }
   else if ((result = pthread_mutex_lock (&s->lock)) == 0)
     {
+      /* See sem_destroy.c
+       */
+      if (*sem == NULL)
+        {
+          (void) pthread_mutex_unlock (&s->lock);
+          result = EINVAL;
+          return -1;
+        }
+
       if (s->value < SEM_VALUE_MAX)
 	{
 #ifdef NEED_SEM
diff --git a/sem_post_multiple.c b/sem_post_multiple.c
index fa950bd..3d1e4ef 100644
--- a/sem_post_multiple.c
+++ b/sem_post_multiple.c
@@ -86,6 +86,15 @@ sem_post_multiple (sem_t * sem, int count)
     }
   else if ((result = pthread_mutex_lock (&s->lock)) == 0)
     {
+      /* See sem_destroy.c
+       */
+      if (*sem == NULL)
+        {
+          (void) pthread_mutex_unlock (&s->lock);
+          result = EINVAL;
+          return -1;
+        }
+
       if (s->value <= (SEM_VALUE_MAX - count))
 	{
 	  waiters = -s->value;
diff --git a/sem_timedwait.c b/sem_timedwait.c
index deefa6e..52146b4 100644
--- a/sem_timedwait.c
+++ b/sem_timedwait.c
@@ -137,6 +137,8 @@ sem_timedwait (sem_t * sem, const struct timespec *abstime)
   int result = 0;
   sem_t s = *sem;
 
+  pthread_testcancel();
+
   if (sem == NULL)
     {
       result = EINVAL;
@@ -157,11 +159,20 @@ sem_timedwait (sem_t * sem, const struct timespec *abstime)
 	  milliseconds = ptw32_relmillisecs (abstime);
 	}
 
-      pthread_testcancel();
-
       if ((result = pthread_mutex_lock (&s->lock)) == 0)
 	{
-	  int v = --s->value;
+	  int v;
+
+	  /* See sem_destroy.c
+	   */
+	  if (*sem == NULL)
+	    {
+	      (void) pthread_mutex_unlock (&s->lock);
+	      errno = EINVAL;
+	      return -1;
+	    }
+
+	  v = --s->value;
 	  (void) pthread_mutex_unlock (&s->lock);
 
 	  if (v < 0)
@@ -192,6 +203,13 @@ sem_timedwait (sem_t * sem, const struct timespec *abstime)
 
 	      if (!timedout && pthread_mutex_lock (&s->lock) == 0)
 	        {
+        	  if (*sem == NULL)
+        	    {
+        	      (void) pthread_mutex_unlock (&s->lock);
+        	      errno = EINVAL;
+        	      return -1;
+        	    }
+
 	          if (s->leftToUnblock > 0)
 	            {
 		      --s->leftToUnblock;
diff --git a/sem_trywait.c b/sem_trywait.c
index 4d1451d..63614ba 100644
--- a/sem_trywait.c
+++ b/sem_trywait.c
@@ -85,6 +85,15 @@ sem_trywait (sem_t * sem)
     }
   else if ((result = pthread_mutex_lock (&s->lock)) == 0)
     {
+      /* See sem_destroy.c
+       */
+     if (*sem == NULL)
+        {
+          (void) pthread_mutex_unlock (&s->lock);
+          errno = EINVAL;
+          return -1;
+        }
+
       if (s->value > 0)
 	{
 	  s->value--;
diff --git a/sem_wait.c b/sem_wait.c
index 05d7326..d39d2b4 100644
--- a/sem_wait.c
+++ b/sem_wait.c
@@ -54,12 +54,13 @@ ptw32_sem_wait_cleanup(void * sem)
   if (pthread_mutex_lock (&s->lock) == 0)
     {
       /*
+       * If sema is destroyed do nothing, otherwise:-
        * If the sema is posted between us being cancelled and us locking
        * the sema again above then we need to consume that post but cancel
        * anyway. If we don't get the semaphore we indicate that we're no
        * longer waiting.
        */
-      if (!(WaitForSingleObject(s->sem, 0) == WAIT_OBJECT_0))
+      if (*((sem_t *)sem) != NULL && !(WaitForSingleObject(s->sem, 0) == WAIT_OBJECT_0))
 	{
 	  ++s->value;
 #ifdef NEED_SEM
@@ -112,20 +113,28 @@ sem_wait (sem_t * sem)
   int result = 0;
   sem_t s = *sem;
 
+  pthread_testcancel();
+
   if (s == NULL)
     {
       result = EINVAL;
     }
   else
     {
-
-      /* Faster to test before adjusting the count */
-      pthread_testcancel();
-
       if ((result = pthread_mutex_lock (&s->lock)) == 0)
 	{
-	  int v = --s->value;
+	  int v;
 
+	  /* See sem_destroy.c
+	   */
+	  if (*sem == NULL)
+	    {
+	      (void) pthread_mutex_unlock (&s->lock);
+	      errno = EINVAL;
+	      return -1;
+	    }
+
+          v = --s->value;
 	  (void) pthread_mutex_unlock (&s->lock);
 
 	  if (v < 0)
@@ -136,6 +145,7 @@ sem_wait (sem_t * sem)
 	      /* Must wait */
 	      pthread_cleanup_push(ptw32_sem_wait_cleanup, (void *) s);
 	      result = pthreadCancelableWait (s->sem);
+	      /* Cleanup if we're canceled or on any other error */
 	      pthread_cleanup_pop(result);
 #ifdef _MSC_VER
 #pragma inline_depth()
@@ -145,6 +155,13 @@ sem_wait (sem_t * sem)
 
 	  if (!result && pthread_mutex_lock (&s->lock) == 0)
 	    {
+	      if (*sem == NULL)
+	        {
+	          (void) pthread_mutex_unlock (&s->lock);
+	          errno = EINVAL;
+	          return -1;
+	        }
+
 	      if (s->leftToUnblock > 0)
 		{
 		  --s->leftToUnblock;
diff --git a/tests/Bmakefile b/tests/Bmakefile
index c1c5805..9a2c2b4 100644
--- a/tests/Bmakefile
+++ b/tests/Bmakefile
@@ -95,7 +95,7 @@ PASSES=   loadfree.pass \
 	  once1.pass  once2.pass  once3.pass  once4.pass  \
 	  self2.pass  \
 	  cancel1.pass  cancel2.pass  \
-	  semaphore4.pass  semaphore4t.pass  \
+	  semaphore4.pass  semaphore4t.pass  semaphore5.pass  \
 	  barrier1.pass  barrier2.pass  barrier3.pass  barrier4.pass  barrier5.pass  \
 	  tsd1.pass  tsd2.pass  delay1.pass  delay2.pass  eyal1.pass  \
 	  condvar3.pass  condvar3_1.pass  condvar3_2.pass  condvar3_3.pass  \
@@ -337,6 +337,7 @@ semaphore2.pass:
 semaphore3.pass: semaphore2.pass
 semaphore4.pass: semaphore3.pass cancel1.pass
 semaphore4t.pass: semaphore4.pass
+semaphore5.pass: semaphore4.pass
 sizes.pass:
 spin1.pass:
 spin2.pass: spin1.pass
diff --git a/tests/GNUmakefile b/tests/GNUmakefile
index 31a514e..1762b6c 100644
--- a/tests/GNUmakefile
+++ b/tests/GNUmakefile
@@ -89,7 +89,7 @@ TESTS	= sizes loadfree \
 	  count1 \
 	  once1 once2 once3 once4 self2 \
 	  cancel1 cancel2 \
-	  semaphore4 semaphore4t \
+	  semaphore4 semaphore4t semaphore5 \
 	  barrier1 barrier2 barrier3 barrier4 barrier5 \
 	  tsd1 tsd2 delay1 delay2 eyal1 \
 	  condvar3 condvar3_1 condvar3_2 condvar3_3 \
@@ -302,6 +302,7 @@ semaphore2.pass:
 semaphore3.pass: semaphore2.pass
 semaphore4.pass: semaphore3.pass cancel1.pass
 semaphore4t.pass: semaphore4.pass
+semaphore5.pass: semaphore4.pass
 sizes.pass:
 spin1.pass:
 spin2.pass: spin1.pass
diff --git a/tests/Makefile b/tests/Makefile
index a1af003..69dc39c 100644
--- a/tests/Makefile
+++ b/tests/Makefile
@@ -99,7 +99,7 @@ PASSES= sizes.pass  loadfree.pass \
 	  once1.pass  once2.pass  once3.pass  once4.pass  \
 	  self2.pass  \
 	  cancel1.pass  cancel2.pass  \
-	  semaphore4.pass  semaphore4t.pass  \
+	  semaphore4.pass  semaphore4t.pass  semaphore5.pass  \
 	  barrier1.pass  barrier2.pass  barrier3.pass  barrier4.pass  barrier5.pass  \
 	  tsd1.pass  tsd2.pass  delay1.pass  delay2.pass  eyal1.pass  \
 	  condvar3.pass  condvar3_1.pass  condvar3_2.pass  condvar3_3.pass  \
@@ -391,6 +391,7 @@ semaphore2.pass:
 semaphore3.pass: semaphore2.pass
 semaphore4.pass: semaphore3.pass cancel1.pass
 semaphore4t.pass: semaphore4.pass
+semaphore5.pass: semaphore4.pass
 sizes.pass:
 spin1.pass:
 spin2.pass: spin1.pass
diff --git a/tests/Wmakefile b/tests/Wmakefile
index 06a755c..83cd34b 100644
--- a/tests/Wmakefile
+++ b/tests/Wmakefile
@@ -95,7 +95,7 @@ PASSES	= sizes.pass  loadfree.pass &
 	  once1.pass  once2.pass  once3.pass  once4.pass  tsd1.pass  &
 	  self2.pass  &
 	  cancel1.pass  cancel2.pass  &
-	  semaphore4.pass semaphore4t.pass  &
+	  semaphore4.pass semaphore4t.pass semaphore5.pass &
 	  delay1.pass  delay2.pass  eyal1.pass  &
 	  condvar3.pass  condvar3_1.pass  condvar3_2.pass  condvar3_3.pass  &
 	  condvar4.pass  condvar5.pass  condvar6.pass  &
@@ -333,6 +333,7 @@ semaphore2.pass:
 semaphore3.pass: semaphore2.pass
 semaphore4.pass: semaphore3.pass cancel1.pass
 semaphore4t.pass: semaphore4.pass
+semaphore5.pass: semaphore4.pass
 sizes.pass:
 spin1.pass:
 spin2.pass: spin1.pass
diff --git a/tests/once3.c b/tests/once3.c
index 51d2daa..981bbf7 100644
--- a/tests/once3.c
+++ b/tests/once3.c
@@ -34,7 +34,7 @@
  * --------------------------------------------------------------------------
  *
  * Create several pthread_once objects and channel several threads
- * through each. Make the init_routine cancelable and cancel them
+ * through each. Make the init_routine cancelable and cancel them with
  * waiters waiting.
  *
  * Depends on API functions:
@@ -45,6 +45,8 @@
  *      pthread_once()
  */
 
+#define ASSERT_TRACE
+
 #include "test.h"
 
 #define NUM_THREADS 100 /* Targeting each once control */
@@ -66,6 +68,7 @@ myfunc(void)
 {
   EnterCriticalSection(&numOnce.cs);
   numOnce.i++;
+  assert(numOnce.i > 0);
   LeaveCriticalSection(&numOnce.cs);
   /* Simulate slow once routine so that following threads pile up behind it */
   Sleep(10);
@@ -78,11 +81,11 @@ mythread(void * arg)
 {
   /*
    * Cancel every thread. These threads are deferred cancelable only, so
-   * only the thread performing the init_routine will see it (there are
+   * only the thread performing the once routine (my_func) will see it (there are
    * no other cancelation points here). The result will be that every thread
-   * eventually cancels only when it becomes the new initter.
+   * eventually cancels only when it becomes the new once thread.
    */
-  pthread_cancel(pthread_self());
+  assert(pthread_cancel(pthread_self()) == 0);
   assert(pthread_once(&once[(int) arg], myfunc) == 0);
   EnterCriticalSection(&numThreads.cs);
   numThreads.i++;