From ec8ad17e258e9758c10c5266ed7d4c4291b3a43b Mon Sep 17 00:00:00 2001 From: Joe Mistachkin Date: Sun, 17 May 2015 18:03:15 +0000 Subject: Draft fix for a potential race condition in the new Tcl_MutexUnlockAndFinalize API. Not yet tested. --- generic/tclInt.h | 2 ++ generic/tclThread.c | 11 ++++++--- unix/tclUnixThrd.c | 67 +++++++++++++++++++++++++++++++++++++++++++++++++++++ win/tclWinThrd.c | 63 +++++++++++++++++++++++++++++++++++++++++++++++++ 4 files changed, 140 insertions(+), 3 deletions(-) diff --git a/generic/tclInt.h b/generic/tclInt.h index 3f84717..bcf2355 100644 --- a/generic/tclInt.h +++ b/generic/tclInt.h @@ -3059,6 +3059,8 @@ MODULE_SCOPE void TclpInitUnlock(void); MODULE_SCOPE Tcl_Obj * TclpObjListVolumes(void); MODULE_SCOPE void TclpMasterLock(void); MODULE_SCOPE void TclpMasterUnlock(void); +MODULE_SCOPE void TclpMutexLock(void); +MODULE_SCOPE void TclpMutexUnlock(void); MODULE_SCOPE int TclpMatchFiles(Tcl_Interp *interp, char *separators, Tcl_DString *dirPtr, char *pattern, char *tail); MODULE_SCOPE int TclpObjNormalizePath(Tcl_Interp *interp, diff --git a/generic/tclThread.c b/generic/tclThread.c index 64e956a..db326ad 100644 --- a/generic/tclThread.c +++ b/generic/tclThread.c @@ -305,12 +305,17 @@ void Tcl_MutexUnlockAndFinalize( Tcl_Mutex *mutexPtr) { + Tcl_Mutex mutex; TclpMasterLock(); + TclpMutexLock(); #ifdef TCL_THREADS - Tcl_MutexUnlock(mutexPtr); - TclpFinalizeMutex(mutexPtr); + mutex = *mutexPtr; + *mutexPtr = NULL; /* Force it to be created again. */ + Tcl_MutexUnlock(&mutex); + TclpFinalizeMutex(&mutex); #endif - ForgetSyncObject(mutexPtr, &mutexRecord); + ForgetSyncObject(&mutex, &mutexRecord); + TclpMutexUnlock(); TclpMasterUnlock(); } diff --git a/unix/tclUnixThrd.c b/unix/tclUnixThrd.c index d34bc88..42b6bda 100644 --- a/unix/tclUnixThrd.c +++ b/unix/tclUnixThrd.c @@ -45,6 +45,13 @@ static pthread_mutex_t allocLock = PTHREAD_MUTEX_INITIALIZER; static pthread_mutex_t *allocLockPtr = &allocLock; /* + * The mutexLock serializes Tcl_MutexLock. This is necessary to prevent + * races when finalizing a mutex that some other thread may want to lock. + */ + +static pthread_mutex_t mutexLock = PTHREAD_MUTEX_INITIALIZER; + +/* * These are for the critical sections inside this file. */ @@ -365,6 +372,58 @@ TclpMasterUnlock(void) /* *---------------------------------------------------------------------- * + * TclpMutexLock + * + * This procedure is used to grab a lock that serializes locking + * another mutex. + * + * Results: + * None. + * + * Side effects: + * None. + * + *---------------------------------------------------------------------- + */ + +void +TclpMutexLock(void) +{ +#ifdef TCL_THREADS + pthread_mutex_lock(&mutexLock); +#endif +} + + +/* + *---------------------------------------------------------------------- + * + * TclpMutexUnlock + * + * This procedure is used to release a lock that serializes locking + * another mutex. + * + * Results: + * None. + * + * Side effects: + * None. + * + *---------------------------------------------------------------------- + */ + +void +TclpMutexUnlock(void) +{ +#ifdef TCL_THREADS + pthread_mutex_unlock(&mutexLock); +#endif +} + + +/* + *---------------------------------------------------------------------- + * * Tcl_GetAllocMutex * * This procedure returns a pointer to a statically initialized mutex for @@ -421,6 +480,8 @@ Tcl_MutexLock( { pthread_mutex_t *pmutexPtr; +retry: + if (*mutexPtr == NULL) { MASTER_LOCK; if (*mutexPtr == NULL) { @@ -435,8 +496,14 @@ Tcl_MutexLock( } MASTER_UNLOCK; } + TclpMutexLock(); pmutexPtr = *((pthread_mutex_t **)mutexPtr); + if (pmutexPtr == NULL) { + TclpMutexUnlock(); + goto retry; + } pthread_mutex_lock(pmutexPtr); + TclpMutexUnlock(); } /* diff --git a/win/tclWinThrd.c b/win/tclWinThrd.c index 1c9d483..4748818 100644 --- a/win/tclWinThrd.c +++ b/win/tclWinThrd.c @@ -57,6 +57,13 @@ static int allocOnce = 0; #endif /* TCL_THREADS */ /* + * The mutexLock serializes Tcl_MutexLock. This is necessary to prevent + * races when finalizing a mutex that some other thread may want to lock. + */ + +static CRITICAL_SECTION mutexLock; + +/* * The joinLock serializes Create- and ExitThread. This is necessary to * prevent a race where a new joinable thread exits before the creating thread * had the time to create the necessary data structures in the emulation @@ -369,6 +376,7 @@ TclpInitLock(void) */ init = 1; + InitializeCriticalSection(&mutexLock); InitializeCriticalSection(&joinLock); InitializeCriticalSection(&initLock); InitializeCriticalSection(&masterLock); @@ -464,6 +472,52 @@ TclpMasterUnlock(void) /* *---------------------------------------------------------------------- * + * TclpMutexLock + * + * This procedure is used to grab a lock that serializes locking + * another mutex. + * + * Results: + * None. + * + * Side effects: + * None. + * + *---------------------------------------------------------------------- + */ + +void +TclpMutexLock(void) +{ + EnterCriticalSection(&mutexLock); +} + +/* + *---------------------------------------------------------------------- + * + * TclpMutexUnlock + * + * This procedure is used to release a lock that serializes locking + * another mutex. + * + * Results: + * None. + * + * Side effects: + * None. + * + *---------------------------------------------------------------------- + */ + +void +TclpMutexUnlock(void) +{ + LeaveCriticalSection(&mutexLock); +} + +/* + *---------------------------------------------------------------------- + * * Tcl_GetAllocMutex * * This procedure returns a pointer to a statically initialized mutex for @@ -516,6 +570,7 @@ void TclFinalizeLock(void) { MASTER_LOCK; + DeleteCriticalSection(&mutexLock); DeleteCriticalSection(&joinLock); /* @@ -569,6 +624,8 @@ Tcl_MutexLock( { CRITICAL_SECTION *csPtr; +retry: + if (*mutexPtr == NULL) { MASTER_LOCK; @@ -584,8 +641,14 @@ Tcl_MutexLock( } MASTER_UNLOCK; } + TclpMutexLock(); csPtr = *((CRITICAL_SECTION **)mutexPtr); + if (csPtr == NULL) { + TclpMutexUnlock(); + goto retry; + } EnterCriticalSection(csPtr); + TclpMutexUnlock(); } /* -- cgit v0.12