Draft fix for a potential race condition in the new Tcl_MutexUnlockAndFinalize API. Not yet tested.

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();
 }
 
 /*