summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorhobbs <hobbs>2003-05-11 01:23:12 (GMT)
committerhobbs <hobbs>2003-05-11 01:23:12 (GMT)
commit1df1a5e7e8cd46a3c9858b816e3f8836f6f39c18 (patch)
tree505c10f89189320d41b9fde3309e6a7bc4a79cc5
parent6582ac0a0d302dd8664f42edc0336906c7752569 (diff)
downloadtcl-1df1a5e7e8cd46a3c9858b816e3f8836f6f39c18.zip
tcl-1df1a5e7e8cd46a3c9858b816e3f8836f6f39c18.tar.gz
tcl-1df1a5e7e8cd46a3c9858b816e3f8836f6f39c18.tar.bz2
* generic/tclIOUtil.c: ensure cd is thread-safe.
[Bug #710642] (vasiljevic)
-rw-r--r--ChangeLog3
-rw-r--r--generic/tclIOUtil.c170
2 files changed, 105 insertions, 68 deletions
diff --git a/ChangeLog b/ChangeLog
index 28b8f5c..cc7f5c4 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,5 +1,8 @@
2003-05-10 Jeff Hobbs <jeffh@ActiveState.com>
+ * generic/tclIOUtil.c: ensure cd is thread-safe.
+ [Bug #710642] (vasiljevic)
+
* win/tclWinSerial.c (SerialCloseProc): correct mem leak on
closing a Windows serial port [Bug #718002] (schroedter)
diff --git a/generic/tclIOUtil.c b/generic/tclIOUtil.c
index 6464a5b..840c431 100644
--- a/generic/tclIOUtil.c
+++ b/generic/tclIOUtil.c
@@ -17,7 +17,7 @@
* See the file "license.terms" for information on usage and redistribution
* of this file, and for a DISCLAIMER OF ALL WARRANTIES.
*
- * RCS: @(#) $Id: tclIOUtil.c,v 1.77.2.2 2003/04/14 15:45:49 vincentdarley Exp $
+ * RCS: @(#) $Id: tclIOUtil.c,v 1.77.2.3 2003/05/11 01:23:13 hobbs Exp $
*/
#include "tclInt.h"
@@ -488,8 +488,21 @@ TCL_DECLARE_MUTEX(filesystemMutex)
* This is protected by the cwdMutex below.
*/
static Tcl_Obj* cwdPathPtr = NULL;
+static int cwdPathEpoch = 0;
TCL_DECLARE_MUTEX(cwdMutex)
+/*
+ * This structure holds per-thread private copy of the
+ * current directory maintained by the global cwdPathPtr.
+ */
+typedef struct ThreadSpecificData {
+ int initialized;
+ int cwdPathEpoch;
+ Tcl_Obj *cwdPathPtr;
+} ThreadSpecificData;
+
+Tcl_ThreadDataKey dataKey;
+
/*
* Declare fallback support function and
* information for Tcl_FSLoadFile
@@ -514,21 +527,48 @@ typedef struct FsDivertLoad {
/* Now move on to the basic filesystem implementation */
+static void
+FsThrExitProc(cd)
+ ClientData cd;
+{
+ ThreadSpecificData *tsdPtr = (ThreadSpecificData*)cd;
+ if (tsdPtr->cwdPathPtr != NULL) {
+ Tcl_DecrRefCount(tsdPtr->cwdPathPtr);
+ }
+}
int
TclFSCwdPointerEquals(objPtr)
Tcl_Obj* objPtr;
{
+ ThreadSpecificData *tsdPtr = TCL_TSD_INIT(&dataKey);
+
Tcl_MutexLock(&cwdMutex);
- if (cwdPathPtr == objPtr) {
- Tcl_MutexUnlock(&cwdMutex);
- return 1;
- } else {
- Tcl_MutexUnlock(&cwdMutex);
- return 0;
+ if (tsdPtr->initialized == 0) {
+ Tcl_CreateThreadExitHandler(FsThrExitProc, (ClientData)tsdPtr);
+ tsdPtr->initialized = 1;
+ }
+ if (tsdPtr->cwdPathPtr == NULL) {
+ if (cwdPathPtr == NULL) {
+ tsdPtr->cwdPathPtr = NULL;
+ } else {
+ tsdPtr->cwdPathPtr = Tcl_DuplicateObj(cwdPathPtr);
+ Tcl_IncrRefCount(tsdPtr->cwdPathPtr);
+ }
+ tsdPtr->cwdPathEpoch = cwdPathEpoch;
+ } else if (tsdPtr->cwdPathEpoch != cwdPathEpoch) {
+ Tcl_DecrRefCount(tsdPtr->cwdPathPtr);
+ if (cwdPathPtr == NULL) {
+ tsdPtr->cwdPathPtr = NULL;
+ } else {
+ tsdPtr->cwdPathPtr = Tcl_DuplicateObj(cwdPathPtr);
+ Tcl_IncrRefCount(tsdPtr->cwdPathPtr);
+ }
}
-}
+ Tcl_MutexUnlock(&cwdMutex);
+ return (tsdPtr->cwdPathPtr == objPtr);
+}
static FilesystemRecord*
FsGetIterator(void) {
@@ -552,6 +592,44 @@ FsReleaseIterator(void) {
Tcl_MutexUnlock(&filesystemMutex);
}
+static void
+FsUpdateCwd(cwdObj)
+ Tcl_Obj *cwdObj;
+{
+ int len;
+ char *str = NULL;
+ ThreadSpecificData *tsdPtr = TCL_TSD_INIT(&dataKey);
+
+ if (cwdObj != NULL) {
+ str = Tcl_GetStringFromObj(cwdObj, &len);
+ }
+
+ Tcl_MutexLock(&cwdMutex);
+ if (cwdPathPtr != NULL) {
+ Tcl_DecrRefCount(cwdPathPtr);
+ }
+ if (cwdObj == NULL) {
+ cwdPathPtr = NULL;
+ } else {
+ /* This must be stored as string obj! */
+ cwdPathPtr = Tcl_NewStringObj(str, len);
+ Tcl_IncrRefCount(cwdPathPtr);
+ }
+ cwdPathEpoch++;
+ tsdPtr->cwdPathEpoch = cwdPathEpoch;
+ Tcl_MutexUnlock(&cwdMutex);
+
+ if (tsdPtr->cwdPathPtr) {
+ Tcl_DecrRefCount(tsdPtr->cwdPathPtr);
+ }
+ if (cwdObj == NULL) {
+ tsdPtr->cwdPathPtr = NULL;
+ } else {
+ tsdPtr->cwdPathPtr = Tcl_NewStringObj(str, len);
+ Tcl_IncrRefCount(tsdPtr->cwdPathPtr);
+ }
+}
+
/*
*----------------------------------------------------------------------
*
@@ -583,6 +661,7 @@ TclFinalizeFilesystem()
if (cwdPathPtr != NULL) {
Tcl_DecrRefCount(cwdPathPtr);
cwdPathPtr = NULL;
+ cwdPathEpoch = 0;
}
/*
@@ -2203,8 +2282,10 @@ Tcl_FSFileAttrsSet(interp, index, pathPtr, objPtr)
* should therefore ensure they only access the cwd through this
* function to avoid confusion.
*
- * If a global cwdPathPtr already exists, it is returned, subject
- * to a synchronisation attempt in that cwdPathPtr's fs.
+ * If a global cwdPathPtr already exists, it is cached in the thread's
+ * private data structures and reference to the cached copy is returned,
+ * subject to a synchronisation attempt in that cwdPathPtr's fs.
+ *
* Otherwise, the chain of functions that have been "inserted"
* into the filesystem will be called in succession until either a
* value other than NULL is returned, or the entire list is
@@ -2218,13 +2299,6 @@ Tcl_FSFileAttrsSet(interp, index, pathPtr, objPtr)
*
* The result already has its refCount incremented for the caller.
* When it is no longer needed, that refCount should be decremented.
- * This is needed for thread-safety purposes, to allow multiple
- * threads to access this and related functions, while ensuring the
- * results are always valid.
- *
- * Of course it is probably a bad idea for multiple threads to
- * be *setting* the cwd anyway, but we can at least try to
- * help the case of multiple reads with occasional sets.
*
* Side effects:
* Various objects may be freed and allocated.
@@ -2236,7 +2310,7 @@ Tcl_Obj*
Tcl_FSGetCwd(interp)
Tcl_Interp *interp;
{
- Tcl_Obj *cwdToReturn;
+ ThreadSpecificData *tsdPtr = TCL_TSD_INIT(&dataKey);
if (TclFSCwdPointerEquals(NULL)) {
FilesystemRecord *fsRecPtr;
@@ -2282,14 +2356,7 @@ Tcl_FSGetCwd(interp)
* we'll always be in the 'else' branch below which
* is simpler.
*/
- Tcl_MutexLock(&cwdMutex);
- /* Just in case the pointer has been set by another
- * thread between now and the test above */
- if (cwdPathPtr != NULL) {
- Tcl_DecrRefCount(cwdPathPtr);
- }
- cwdPathPtr = norm;
- Tcl_MutexUnlock(&cwdMutex);
+ FsUpdateCwd(norm);
}
Tcl_DecrRefCount(retVal);
}
@@ -2301,7 +2368,7 @@ Tcl_FSGetCwd(interp)
* allows an error to be thrown if, say, the permissions on
* that directory have changed.
*/
- Tcl_Filesystem *fsPtr = Tcl_FSGetFileSystemForPath(cwdPathPtr);
+ Tcl_Filesystem *fsPtr = Tcl_FSGetFileSystemForPath(tsdPtr->cwdPathPtr);
/*
* If the filesystem couldn't be found, or if no cwd function
* exists for this filesystem, then we simply assume the cached
@@ -2324,7 +2391,7 @@ Tcl_FSGetCwd(interp)
*/
if (norm == NULL) {
/* Do nothing */
- } else if (Tcl_FSEqualPaths(cwdPathPtr, norm)) {
+ } else if (Tcl_FSEqualPaths(tsdPtr->cwdPathPtr, norm)) {
/*
* If the paths were equal, we can be more
* efficient and retain the old path object
@@ -2334,42 +2401,22 @@ Tcl_FSGetCwd(interp)
*/
Tcl_DecrRefCount(norm);
} else {
- /* The cwd has in fact changed, so we must
- * lock down the cwdMutex to modify. */
- Tcl_MutexLock(&cwdMutex);
- Tcl_DecrRefCount(cwdPathPtr);
- cwdPathPtr = norm;
- Tcl_MutexUnlock(&cwdMutex);
+ FsUpdateCwd(norm);
}
Tcl_DecrRefCount(retVal);
} else {
- /* The 'cwd' function returned an error, so we
- * reset the cwd after locking down the mutex. */
- Tcl_MutexLock(&cwdMutex);
- Tcl_DecrRefCount(cwdPathPtr);
- cwdPathPtr = NULL;
- Tcl_MutexUnlock(&cwdMutex);
+ /* The 'cwd' function returned an error; reset the cwd */
+ FsUpdateCwd(NULL);
}
}
}
}
- /*
- * The paths all eventually fall through to here. Note that
- * we use a bunch of separate mutex locks throughout this
- * code to help prevent deadlocks between threads. Really
- * the only weirdness will arise if multiple threads are setting
- * and reading the cwd, and that behaviour is always going to be
- * a little suspect.
- */
- Tcl_MutexLock(&cwdMutex);
- cwdToReturn = cwdPathPtr;
- if (cwdToReturn != NULL) {
- Tcl_IncrRefCount(cwdToReturn);
+ if (tsdPtr->cwdPathPtr != NULL) {
+ Tcl_IncrRefCount(tsdPtr->cwdPathPtr);
}
- Tcl_MutexUnlock(&cwdMutex);
- return (cwdToReturn);
+ return tsdPtr->cwdPathPtr;
}
/*
@@ -2446,20 +2493,7 @@ Tcl_FSChdir(pathPtr)
if (normDirName == NULL) {
return TCL_ERROR;
}
- /*
- * We will be adding a reference to this object when
- * we store it in the cwdPathPtr.
- */
- Tcl_IncrRefCount(normDirName);
- /* Get a lock on the cwd while we modify it */
- Tcl_MutexLock(&cwdMutex);
- /* Free up the previous cwd we stored */
- if (cwdPathPtr != NULL) {
- Tcl_DecrRefCount(cwdPathPtr);
- }
- /* Now remember the current cwd */
- cwdPathPtr = normDirName;
- Tcl_MutexUnlock(&cwdMutex);
+ FsUpdateCwd(normDirName);
}
} else {
Tcl_SetErrno(ENOENT);