diff options
| -rw-r--r-- | ChangeLog | 69 | ||||
| -rw-r--r-- | implement.h | 16 | ||||
| -rw-r--r-- | pthread.h | 10 | ||||
| -rw-r--r-- | pthread_once.c | 162 | ||||
| -rw-r--r-- | tests/Bmakefile | 2 | ||||
| -rw-r--r-- | tests/GNUmakefile | 2 | ||||
| -rw-r--r-- | tests/Makefile | 2 | ||||
| -rw-r--r-- | tests/Wmakefile | 2 |
8 files changed, 86 insertions, 179 deletions
@@ -1,65 +1,20 @@ 2005-03-14 Ross Johnson <ross at callisto.canberra.edu.au> - * pthread_once.c (pthread_once): Downgrade interlocked operations to simple - memory operations where these are protected by the critical section; edit - comments. - -2005-03-13 Ross Johnson <rpj at callisto.canberra.edu.au> - - * pthread_once.c (pthread_once): Completely redesigned; a change was - required to the ABI (pthread_once_t_), and resulting in a version - compatibility index increment. - - NOTES: - The design (based on pseudo code contributed by Gottlob Frege) avoids - creating a kernel object if there is no contention. See URL for details:- - http://sources.redhat.com/ml/pthreads-win32/2005/msg00029.html - This uses late initialisation similar to the technique already used for - pthreads-win32 mutexes and semaphores (from Alexander Terekhov). - - The subsequent cancelation cleanup additions (by rpj) could not be implemented - without sacrificing some of the efficiency in Gottlob's design. In particular, - although each once_control uses it's own event to block on, a global CS is - required to manage it - since the event must be either re-usable or - re-creatable under cancelation. This is not needed in the non-cancelable - design because it is able to mark the event as closed (forever). - - When uncontested, a CS operation is equivalent to an Interlocked operation - in speed. So, in the final design with cancelability, an uncontested - once_control operation involves a minimum of five interlocked operations - (including the LeaveCS operation). - - ALTERNATIVES: - An alternative design from Alexander Terekhov proposed using a named mutex, - as sketched below:- - - if (!once_control) { // May be in TLS - named_mutex::guard guard(&once_control2); - if (!once_control2) { - <init> - once_control2 = true; - } - once_control = true; - } - - A more detailed description of this can be found here:- - http://groups.yahoo.com/group/boost/message/15442 - - [Although the definition of a suitable PTHREAD_ONCE_INIT precludes use of the - TLS located flag, this is not critical.] + * CVS (snapshot-2005-03-08-fixes): Created branch for + back-porting fixes to snapshot 2005-03-08. - There are three primary concerns though:- - 1) The [named] mutex is 'created' even in the uncontended case. - 2) A system wide unique name must be generated. - 3) Win32 mutexes are VERY slow even in the uncontended case. An uncontested - Win32 mutex lock operation can be 50 (or more) times slower than an - uncontested EnterCS operation. + * pthread_once.c (pthread_once): Add cancellability to the + init_routine; remove cancellability elsewhere (by disabling + cancellability around pthread_cond_wait in particular); + the meaning of the 'done' flag is changed but the ABI doesn't + change as a result. - Ultimately, the named mutex trick is making use of the global locks maintained - by the kernel. + * pthread.h (PTHREAD_ONCE_INIT): Change the 'done' flag value + representation (does not change the value - still zero (0)). - * pthread.h (pthread_once_t_): One flag and an event HANDLE added. - (PTHREAD_ONCE_INIT): Additional values included. + * implement.h (PTW32_ONCE_CLEAR): Defined. + (PTW32_ONCE_DONE): Defined. + (PTW32_ONCE_CANCELLED): Defined. 2005-03-08 Ross Johnson <rpj at callisto.canberra.edu.au>
diff --git a/implement.h b/implement.h index abba13c..69a70c6 100644 --- a/implement.h +++ b/implement.h @@ -318,12 +318,20 @@ struct pthread_rwlockattr_t_ int pshared; }; -enum ptw32_once_state { - PTW32_ONCE_CLEAR = 0x0, - PTW32_ONCE_DONE = 0x1, +/* + * Values stored in once_control->done. + * 'done' use to be just true or false, but we can add cancellability + * of the init_routine by re-using 'done' to store multiple flags + * without changing the ABI. Previously, the initial value of 'done' + * was FALSE (0), and the new initial value is still zero (0). + */ +enum { + PTW32_ONCE_CLEAR = 0x0, + PTW32_ONCE_DONE = 0x1, PTW32_ONCE_CANCELLED = 0x2 }; +/* Global cond+mutex for once_control management */ typedef struct { pthread_cond_t cond; pthread_mutex_t mtx; @@ -483,7 +491,7 @@ extern CRITICAL_SECTION ptw32_cond_list_lock; extern CRITICAL_SECTION ptw32_cond_test_init_lock; extern CRITICAL_SECTION ptw32_rwlock_test_init_lock; extern CRITICAL_SECTION ptw32_spinlock_test_init_lock; -extern CRITICAL_SECTION ptw32_once_event_lock; +extern ptw32_once_control_t ptw32_once_control; #ifdef _UWIN extern int pthread_count; @@ -37,8 +37,8 @@ * See the README file for an explanation of the pthreads-win32 version * numbering scheme and how the DLL is named etc. */ -#define PTW32_VERSION 2,0,0,0 -#define PTW32_VERSION_STRING "2, 0, 0, 0\0" +#define PTW32_VERSION 1,4,0,0 +#define PTW32_VERSION_STRING "1, 4, 0, 0\0" /* There are three implementations of cancel cleanup. * Note that pthread.h is included in both application @@ -673,14 +673,12 @@ enum { * ==================== * ==================== */ -#define PTHREAD_ONCE_INIT { 0, PTW32_FALSE, 0, 0} +#define PTHREAD_ONCE_INIT { 0, -1 } struct pthread_once_t_ { - int state; /* indicates if user function has been executed, or cancelled */ + volatile int done; /* indicates if user function has been executed or cancelled */ int started; - int eventUsers; - HANDLE event; }; diff --git a/pthread_once.c b/pthread_once.c index 1c0e01f..d27b49c 100644 --- a/pthread_once.c +++ b/pthread_once.c @@ -41,21 +41,20 @@ static void ptw32_once_init_routine_cleanup(void * arg) { + int oldCancelState; pthread_once_t * once_control = (pthread_once_t *) arg; - (void) PTW32_INTERLOCKED_EXCHANGE((LPLONG)&once_control->state, (LONG)PTW32_ONCE_CANCELLED); - (void) PTW32_INTERLOCKED_EXCHANGE((LPLONG)&once_control->started, (LONG)PTW32_FALSE); - - EnterCriticalSection(&ptw32_once_event_lock); - if (once_control->event) - { - /* - * There are waiters, wake some up - * We're deliberately not using PulseEvent. It's iffy, and deprecated. - */ - SetEvent(once_control->event); - } - LeaveCriticalSection(&ptw32_once_event_lock); + (void) PTW32_INTERLOCKED_EXCHANGE((LPLONG)&once_control->done, (LONG)PTW32_ONCE_CANCELLED); + (void) PTW32_INTERLOCKED_EXCHANGE((LPLONG)&once_control->started, -1L); + /* + * Wake everyone up. + * + * Holding the mutex during the broadcast prevents threads being left + * behind waiting. + */ + (void) pthread_mutex_lock(&ptw32_once_control.mtx); + (void) pthread_cond_broadcast(&ptw32_once_control.cond); + (void) pthread_mutex_unlock(&ptw32_once_control.mtx); } @@ -75,11 +74,6 @@ pthread_once (pthread_once_t * once_control, void (*init_routine) (void)) * access is controlled by the pthread_once_t control * key. * - * pthread_once() is not a cancelation point, but the init_routine - * can be. If it's cancelled then the effect on the once_control is - * as if pthread_once had never been entered. - * - * * PARAMETERS * once_control * pointer to an instance of pthread_once_t @@ -99,6 +93,7 @@ pthread_once (pthread_once_t * once_control, void (*init_routine) (void)) */ { int result; + int oldCancelState; if (once_control == NULL || init_routine == NULL) { @@ -112,113 +107,64 @@ pthread_once (pthread_once_t * once_control, void (*init_routine) (void)) result = 0; } - while (!(InterlockedExchangeAdd((LPLONG)&once_control->state, 0L) /* Atomic Read */ - & (LONG)PTW32_ONCE_DONE)) + /* + * Use a single global cond+mutex to manage access to all once_control objects. + * Unlike a global mutex on it's own, the global cond+mutex allows faster + * once_controls to overtake slower ones. Spurious wakeups may occur, but + * can be tolerated. + * + * To maintain a separate mutex for each once_control object requires either + * cleaning up the mutex (difficult to synchronise reliably), or leaving it + * around forever. Since we can't make assumptions about how an application might + * employ pthread_once objects, the later is considered to be unacceptable. + * + * Since this is being introduced as a bug fix, the global cond+mtx also avoids + * a change in the ABI, maintaining backwards compatibility. + */ + + while (!InterlockedExchangeAdd((LPLONG)&once_control->done, 0L) /* Full mem barrier read */ + & PTW32_ONCE_DONE) { - if (!PTW32_INTERLOCKED_EXCHANGE((LPLONG)&once_control->started, (LONG)PTW32_TRUE)) + if (PTW32_INTERLOCKED_EXCHANGE((LPLONG) &once_control->started, 0L) == -1) { - /* - * Clear residual state from a cancelled init_routine - * (and DONE still hasn't been set of course). - */ - if (PTW32_INTERLOCKED_EXCHANGE((LPLONG)&once_control->state, (LONG)PTW32_ONCE_CLEAR) - & PTW32_ONCE_CANCELLED) - { - /* - * The previous initter was cancelled. - * We now have a new initter (us) and we need to make the rest wait again. - */ - EnterCriticalSection(&ptw32_once_event_lock); - if (once_control->event) - { - ResetEvent(once_control->event); - } - LeaveCriticalSection(&ptw32_once_event_lock); - - /* - * Any threads entering the wait section and getting out again before - * the CANCELLED state can be cleared and the event is reset will, at worst, just go - * around again or, if they suspend and we (the initter) completes before they resume, - * they will see state == DONE and leave immediately. - */ - } + /* In case the previous initter was cancelled, reset cancelled state */ + (void) PTW32_INTERLOCKED_EXCHANGE((LPLONG)&once_control->done, (LONG)PTW32_ONCE_CLEAR); + +#ifdef _MSC_VER +#pragma inline_depth(0) +#endif - pthread_cleanup_push(ptw32_once_init_routine_cleanup, (void *) once_control); - (*init_routine)(); + pthread_cleanup_push(ptw32_once_init_routine_cleanup, (void*) once_control); + (*init_routine) (); pthread_cleanup_pop(0); - (void) PTW32_INTERLOCKED_EXCHANGE((LPLONG)&once_control->state, (LONG)PTW32_ONCE_DONE); +#ifdef _MSC_VER +#pragma inline_depth() +#endif /* - * we didn't create the event. - * it is only there if there is someone waiting + * Holding the mutex during the broadcast prevents threads being left + * behind waiting. */ - EnterCriticalSection(&ptw32_once_event_lock); - if (once_control->event) - { - SetEvent(once_control->event); - } - LeaveCriticalSection(&ptw32_once_event_lock); + (void) pthread_mutex_lock(&ptw32_once_control.mtx); + once_control->done = PTW32_TRUE; + (void) pthread_cond_broadcast(&ptw32_once_control.cond); + (void) pthread_mutex_unlock(&ptw32_once_control.mtx); } else { - /* - * wait for init. - * while waiting, create an event to wait on - */ - - EnterCriticalSection(&ptw32_once_event_lock); - once_control->eventUsers++; - - /* - * RE CANCELLATION: - * If we are the first thread after the initter thread, and the init_routine is cancelled - * while we're suspended at this point in the code:- - * - state will not get set to PTW32_ONCE_DONE; - * - cleanup will not see an event and cannot set it; - * - therefore, we will eventually resume, create an event and wait on it, maybe forever; - * Remedy: cleanup must set state == CANCELLED before checking for an event, so that - * we will see it and avoid waiting (as for state == DONE). We will go around again and - * we may become the initter. - * If we are still the only other thread when we get to the end of this block, we will - * have closed the event (good). If another thread beats us to be initter, then we will - * re-enter here (good). In case the old event is reused, the event is always reset by - * the new initter after clearing the CANCELLED state, causing any threads that are - * cycling around the loop to wait again. - */ - - if (!once_control->event) - { - once_control->event = CreateEvent(NULL, PTW32_TRUE, PTW32_FALSE, NULL); - } - LeaveCriticalSection(&ptw32_once_event_lock); - - /* - * check 'state' again in case the initting thread has finished or cancelled - * and left before seeing that there was an event to trigger. - * (Now that the event IS created, if init gets finished AFTER this, - * then the event handle is guaranteed to be seen and triggered). - */ - - if (!InterlockedExchangeAdd((LPLONG)&once_control->state, 0L)) /* Atomic Read */ + pthread_setcancelstate(PTHREAD_CANCEL_DISABLE, &oldCancelState); + (void) pthread_mutex_lock(&ptw32_once_control.mtx); + while (!once_control->done) { /* Neither DONE nor CANCELLED */ - (void) WaitForSingleObject(once_control->event, INFINITE); + (void) pthread_cond_wait(&ptw32_once_control.cond, &ptw32_once_control.mtx); } - - /* last one out shut off the lights */ - EnterCriticalSection(&ptw32_once_event_lock); - if (0 == --once_control->eventUsers) - { - /* we were last */ - CloseHandle(once_control->event); - once_control->event = 0; - } - LeaveCriticalSection(&ptw32_once_event_lock); + (void) pthread_mutex_unlock(&ptw32_once_control.mtx); + pthread_setcancelstate(oldCancelState, NULL); } } - /* * Fall through Intentionally */ diff --git a/tests/Bmakefile b/tests/Bmakefile index 924ea06..9943037 100644 --- a/tests/Bmakefile +++ b/tests/Bmakefile @@ -31,7 +31,7 @@ # 59 Temple Place - Suite 330, Boston, MA 02111-1307, USA
#
-DLL_VER = 2
+DLL_VER = 1
CP = copy
RM = erase
diff --git a/tests/GNUmakefile b/tests/GNUmakefile index b8b1c91..08709a8 100644 --- a/tests/GNUmakefile +++ b/tests/GNUmakefile @@ -31,7 +31,7 @@ # 59 Temple Place - Suite 330, Boston, MA 02111-1307, USA # -DLL_VER = 2 +DLL_VER = 1 CP = cp -f MV = mv -f diff --git a/tests/Makefile b/tests/Makefile index 3b75670..dfb0f3c 100644 --- a/tests/Makefile +++ b/tests/Makefile @@ -31,7 +31,7 @@ # 59 Temple Place - Suite 330, Boston, MA 02111-1307, USA
#
-DLL_VER = 2
+DLL_VER = 1
CP = copy
RM = erase
diff --git a/tests/Wmakefile b/tests/Wmakefile index ff54700..653a09a 100644 --- a/tests/Wmakefile +++ b/tests/Wmakefile @@ -32,7 +32,7 @@ #
-DLL_VER = 2
+DLL_VER = 1
.EXTENSIONS:
|