diff options
| -rw-r--r-- | ANNOUNCE | 2 | ||||
| -rw-r--r-- | BUGS | 5 | ||||
| -rw-r--r-- | ChangeLog | 21 | ||||
| -rw-r--r-- | NEWS | 25 | ||||
| -rw-r--r-- | pthread_win32_attach_detach_np.c | 11 | ||||
| -rw-r--r-- | ptw32_MCS_lock.c | 4 | ||||
| -rw-r--r-- | sem_destroy.c | 62 | ||||
| -rw-r--r-- | sem_getvalue.c | 9 | ||||
| -rw-r--r-- | sem_init.c | 2 | ||||
| -rw-r--r-- | sem_post.c | 9 | ||||
| -rw-r--r-- | sem_post_multiple.c | 9 | ||||
| -rw-r--r-- | sem_timedwait.c | 24 | ||||
| -rw-r--r-- | sem_trywait.c | 9 | ||||
| -rw-r--r-- | sem_wait.c | 29 | ||||
| -rw-r--r-- | tests/Bmakefile | 3 | ||||
| -rw-r--r-- | tests/GNUmakefile | 3 | ||||
| -rw-r--r-- | tests/Makefile | 3 | ||||
| -rw-r--r-- | tests/Wmakefile | 3 | ||||
| -rw-r--r-- | tests/once3.c | 11 |
19 files changed, 191 insertions, 53 deletions
@@ -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 @@ -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. @@ -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. @@ -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; @@ -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); @@ -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--; @@ -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++; |