From 5865dd7b74b0b23eb178249bfd5a000cc929f147 Mon Sep 17 00:00:00 2001 From: rpj Date: Tue, 10 Jul 2001 08:38:43 +0000 Subject: Untested revamp of barrier.c. * barrier.c: Still more revamping. The exclusive access mutex isn't really needed so it has been removed and replaced by an InterlockedDecrement(). nSerial has been removed. iStep is now dual-purpose. The process shared attribute is now stored in the barrier struct. * implement.h (pthread_barrier_t_): Lost some/gained one elements. * private.c (ptw32_threadStart): Removed some comments. --- ChangeLog | 11 ++++ barrier.c | 168 +++++++++++++++++++++++++++--------------------------------- implement.h | 3 +- private.c | 13 +---- spin.c | 2 +- 5 files changed, 90 insertions(+), 107 deletions(-) diff --git a/ChangeLog b/ChangeLog index 97ec503..5061a51 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,3 +1,14 @@ +2001-07-10 Ross Johnson + + * barrier.c: Still more revamping. The exclusive access + mutex isn't really needed so it has been removed and replaced + by an InterlockedDecrement(). nSerial has been removed. + iStep is now dual-purpose. The process shared attribute + is now stored in the barrier struct. + * implement.h (pthread_barrier_t_): Lost some/gained one + elements. + * private.c (ptw32_threadStart): Removed some comments. + 2001-07-10 Ross Johnson * barrier.c: Revamped to fix the race condition. Two alternating diff --git a/barrier.c b/barrier.c index e9312c9..3077e9b 100644 --- a/barrier.c +++ b/barrier.c @@ -2,7 +2,7 @@ * barrier.c * * Description: - * This translation unit implements spin locks primitives. + * This translation unit implements barrier primitives. * * Pthreads-win32 - POSIX Threads Library for Win32 * Copyright (C) 1998 @@ -40,9 +40,7 @@ pthread_barrier_init(pthread_barrier_t * barrier, const pthread_barrierattr_t * attr, unsigned int count) { - int pshared = PTHREAD_PROCESS_PRIVATE; pthread_barrier_t b; - pthread_mutexattr_t ma; if (barrier == NULL || count == 0) { @@ -51,33 +49,29 @@ pthread_barrier_init(pthread_barrier_t * barrier, if (NULL != (b = (pthread_barrier_t) calloc(1, sizeof(*b)))) { - if (attr != NULL && *attr != NULL) - { - pshared = (*attr)->pshared; - } + b->pshared = (attr != NULL && *attr != NULL + ? (*attr)->pshared + : PTHREAD_PROCESS_PRIVATE); b->nCurrentBarrierHeight = b->nInitialBarrierHeight = count; b->iStep = 0; - b->nSerial = 0; - if (0 == pthread_mutexattr_init(&ma) && - 0 == pthread_mutexattr_setpshared(&ma, pshared)) + /* + * Two semaphores are used in the same way as two stepping + * stones might be used in crossing a stream. Once all + * threads are safely on one stone, the other stone can + * be moved ahead, and the threads can start moving to it. + * If some threads decide to eat their lunch before moving + * then the other threads have to wait. + */ + if (0 == sem_init(&(b->semBarrierBreeched[0]), b->pshared, 0)) { - if (0 == pthread_mutex_init(&(b->mtxExclusiveAccess), &ma)) + if (0 == sem_init(&(b->semBarrierBreeched[1]), b->pshared, 0)) { - pthread_mutexattr_destroy(&ma); - if (0 == sem_init(&(b->semBarrierBreeched[0]), pshared, 0)) - { - if (0 == sem_init(&(b->semBarrierBreeched[1]), pshared, 0)) - { - *barrier = b; - return 0; - } - (void) sem_destroy(&(b->semBarrierBreeched[0])); - } - (void) pthread_mutex_destroy(&(b->mtxExclusiveAccess)); + *barrier = b; + return 0; } - pthread_mutexattr_destroy(&ma); + (void) sem_destroy(&(b->semBarrierBreeched[0])); } (void) free(b); } @@ -97,34 +91,30 @@ pthread_barrier_destroy(pthread_barrier_t *barrier) } b = *barrier; - - result = pthread_mutex_trylock(&(b->mtxExclusiveAccess)); + *barrier = NULL; - if (0 == result) + if (0 == (result = sem_destroy(&(b->semBarrierBreeched[0])))) { - /* - * FIXME!!! - * The mutex isn't held by another thread but we could still - * be too late invalidating the barrier below since another thread - * may alredy have entered barrier_wait and the check for a valid - * *barrier != NULL. - */ - *barrier = NULL; - (void) pthread_mutex_unlock(&(b->mtxExclusiveAccess)); - - (void) sem_destroy(&(b->semBarrierBreeched[1])); - (void) sem_destroy(&(b->semBarrierBreeched[0])); - (void) pthread_mutex_destroy(&(b->mtxExclusiveAccess)); - (void) free(b); + if (0 == (result = sem_destroy(&(b->semBarrierBreeched[1])))) + { + (void) free(b); + return 0; + } + (void) sem_init(&(b->semBarrierBreeched[0]), + b->pshared, + 0); } + *barrier = b; return(result); } + int pthread_barrier_wait(pthread_barrier_t *barrier) { int result; + int step; pthread_barrier_t b; if (barrier == NULL || *barrier == (pthread_barrier_t) PTW32_OBJECT_INVALID) @@ -133,69 +123,61 @@ pthread_barrier_wait(pthread_barrier_t *barrier) } b = *barrier; + step = b->iStep; - result = pthread_mutex_lock(&(b->mtxExclusiveAccess)); - - if (0 == result) + if (0 == InterlockedDecrement((_LPLONG) &(b->nCurrentBarrierHeight))) { - int step = b->iStep; - unsigned int serial = b->nSerial; + /* Must be done before posting the semaphore. */ + b->nCurrentBarrierHeight = b->nInitialBarrierHeight; - if (0 == --(b->nCurrentBarrierHeight)) + /* + * There is no race condition between the semaphore wait and post + * because we are using two alternating semas and all threads have + * entered barrier_wait and checked nCurrentBarrierHeight before this + * barrier's sema can be posted. Any threads that have not quite + * entered sem_wait below when the multiple_post has completed + * will nevertheless continue through the sema (barrier) + * and will not be left stranded. + */ + if (b->nInitialBarrierHeight > 1) { - (void) pthread_mutex_unlock(&(b->mtxExclusiveAccess)); - /* - * All other threads in this barrier set have at least passed - * through the decrement and test above, so its safe to - * raise the barrier to its full height. - */ - b->iStep = 1 - step; - b->nCurrentBarrierHeight = b->nInitialBarrierHeight; - (void) sem_post_multiple(&(b->semBarrierBreeched[step]), b->nInitialBarrierHeight - 1); + result = sem_post_multiple(&(b->semBarrierBreeched[step]), + b->nInitialBarrierHeight - 1); } - else - { - pthread_t self; - int oldCancelState; - - (void) pthread_mutex_unlock(&(b->mtxExclusiveAccess)); + } + else + { + BOOL switchCancelState; + int oldCancelState; + pthread_t self = pthread_self(); - self = pthread_self(); - - /* - * pthread_barrier_wait() is not a cancelation point - * so temporarily prevent pthreadCancelableWait() from being one. - */ - if (self->cancelType == PTHREAD_CANCEL_DEFERRED) - { - pthread_setcancelstate(PTHREAD_CANCEL_DISABLE, &oldCancelState); - } + /* + * This routine is not a cancelation point, so temporarily + * prevent sem_wait() from being one. + * PTHREAD_CANCEL_ASYNCHRONOUS threads can still be canceled. + */ + switchCancelState = (self->cancelType == PTHREAD_CANCEL_DEFERRED && + 0 == pthread_setcancelstate(PTHREAD_CANCEL_DISABLE, + &oldCancelState)); - /* - * There is no race condition between the semaphore wait and post - * because we are using two alternating semas and all threads have - * entered barrier_wait and checked nCurrentBarrierHeight before this - * barrier's sema can be posted. Any threads that have not quite - * entered sem_wait below when the multiple_post above is called - * will nevertheless continue through the sema (and the barrier). - * We have fulfilled our synchronisation function. - */ - result = sem_wait(&(b->semBarrierBreeched[step])); + result = sem_wait(&(b->semBarrierBreeched[step])); - if (self->cancelType == PTHREAD_CANCEL_DEFERRED) - { - pthread_setcancelstate(oldCancelState, NULL); - } + if (switchCancelState) + { + (void) pthread_setcancelstate(oldCancelState, NULL); } + } - /* - * The first thread to return from the wait will be the - * PTHREAD_BARRIER_SERIAL_THREAD. - */ - result = ((_LONG) serial == InterlockedCompareExchange((_LPLONG) &(b->nSerial), - (_LONG) ((serial + 1) - & 0x7FFFFFFF), - (_LONG) serial) + /* + * The first thread across will be the PTHREAD_BARRIER_SERIAL_THREAD. + * It also sets up the alternate semaphore as the next barrier. + */ + if (0 == result) + { + result = ((_LONG) step == + InterlockedCompareExchange((_LPLONG) &(b->iStep), + (_LONG) (1L - step), + (_LONG) step) ? PTHREAD_BARRIER_SERIAL_THREAD : 0); } diff --git a/implement.h b/implement.h index bc72b3f..080d9a1 100644 --- a/implement.h +++ b/implement.h @@ -189,9 +189,8 @@ struct pthread_spinlock_t_ { struct pthread_barrier_t_ { unsigned int nCurrentBarrierHeight; unsigned int nInitialBarrierHeight; - unsigned int nSerial; int iStep; - pthread_mutex_t mtxExclusiveAccess; + int pshared; sem_t semBarrierBreeched[2]; }; diff --git a/private.c b/private.c index de560c3..a332385 100644 --- a/private.c +++ b/private.c @@ -369,15 +369,10 @@ ptw32_threadStart (void * vthreadParms) * That function may call pthread_exit() or be canceled, which will * be handled by the outer try block. * - * ptw32_terminate() will be called if there is no user supplied function. + * ptw32_terminate() will be called if there is no user + * supplied function. */ - //Original invocation: - //(void) terminate(); - - - //New invocation: - // a) get pointer to the termination function #if defined(_MSC_VER) terminate_function term_func = set_terminate(0); #else @@ -386,14 +381,10 @@ ptw32_threadStart (void * vthreadParms) set_terminate(term_func); - // b) call the termination function (if any) if (term_func != 0) { term_func(); } - // c) if there was no termination function or the termination function did - // not exit thread/process, (we got this far), propagate the exception on! - // (should be caught by the second level try/catch block below) throw; } } diff --git a/spin.c b/spin.c index 26e278c..458bb7a 100644 --- a/spin.c +++ b/spin.c @@ -2,7 +2,7 @@ * spin.c * * Description: - * This translation unit implements spin locks primitives. + * This translation unit implements spin lock primitives. * * Pthreads-win32 - POSIX Threads Library for Win32 * Copyright (C) 1998 -- cgit v1.2.3