diff options
| author | rpj <rpj> | 2005-03-14 01:30:17 +0000 | 
|---|---|---|
| committer | rpj <rpj> | 2005-03-14 01:30:17 +0000 | 
| commit | edff26f320def7562009773ff8f258688d9860f6 (patch) | |
| tree | f168e02fed0e44cac3128f136e2a60b62ea443a1 | |
| parent | 9b477bedafd2450735b78cdedc7af5e090aa8934 (diff) | |
''
| -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:
 | 
