summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
-rw-r--r--ANNOUNCE2
-rw-r--r--BUGS5
-rw-r--r--ChangeLog21
-rw-r--r--NEWS25
-rw-r--r--pthread_win32_attach_detach_np.c11
-rw-r--r--ptw32_MCS_lock.c4
-rw-r--r--sem_destroy.c62
-rw-r--r--sem_getvalue.c9
-rw-r--r--sem_init.c2
-rw-r--r--sem_post.c9
-rw-r--r--sem_post_multiple.c9
-rw-r--r--sem_timedwait.c24
-rw-r--r--sem_trywait.c9
-rw-r--r--sem_wait.c29
-rw-r--r--tests/Bmakefile3
-rw-r--r--tests/GNUmakefile3
-rw-r--r--tests/Makefile3
-rw-r--r--tests/Wmakefile3
-rw-r--r--tests/once3.c11
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++;