summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorvasiljevic <zv@archiware.com>2010-04-03 09:38:47 (GMT)
committervasiljevic <zv@archiware.com>2010-04-03 09:38:47 (GMT)
commit7b471897528b88e318d8e33ab1aafa4648aeea20 (patch)
tree8ba22783bbdc3b0d09a8bc543b10d65a19ef0eb6
parentc5ca5644d35b1fbad9c367e78cc5d38566f584e4 (diff)
downloadtcl-7b471897528b88e318d8e33ab1aafa4648aeea20.zip
tcl-7b471897528b88e318d8e33ab1aafa4648aeea20.tar.gz
tcl-7b471897528b88e318d8e33ab1aafa4648aeea20.tar.bz2
Added VALGRIND define so we can silence helgrind race-report at places
we know we cheated on in order to reduce contention.
-rw-r--r--ChangeLog7
-rw-r--r--generic/tclThreadStorage.c39
2 files changed, 39 insertions, 7 deletions
diff --git a/ChangeLog b/ChangeLog
index 085f07e..43f9f1d 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,10 +1,13 @@
-2010-04-02 Zoran Vasiljevic <vasiljevic@users.sourceforge.net>
+2010-04-03 Zoran Vasiljevic <vasiljevic@users.sourceforge.net>
* generic/tclStringObj.c: (SetStringFromAny): avoid trampling
over the tclEmptyStringRep as it is thread-shared.
* generic/tclThreadStorage.c (ThreadStorageGetHashTable):
- avoid accessing shared table index w/o mutex protection.
+ avoid accessing shared table index w/o mutex protection
+ if VALGRIND defined on compilation time. This rules out
+ helgrind complains about potential race-conditions at
+ that place.
Thanks to Gustaf Neumann for the (hard) work.
diff --git a/generic/tclThreadStorage.c b/generic/tclThreadStorage.c
index d24973f..1afc231 100644
--- a/generic/tclThreadStorage.c
+++ b/generic/tclThreadStorage.c
@@ -8,7 +8,7 @@
* See the file "license.terms" for information on usage and redistribution of
* this file, and for a DISCLAIMER OF ALL WARRANTIES.
*
- * RCS: @(#) $Id: tclThreadStorage.c,v 1.15.2.1 2010/04/02 15:53:53 vasiljevic Exp $
+ * RCS: @(#) $Id: tclThreadStorage.c,v 1.15.2.2 2010/04/03 09:38:47 vasiljevic Exp $
*/
#include "tclInt.h"
@@ -172,7 +172,7 @@ FreeThreadStorageEntry(
* ThreadStorageGetHashTable --
*
* This procedure returns a hash table pointer to be used for thread
- * storage for the specified thread.
+ * storage for the specified thread.
*
* Results:
* A hash table pointer for the specified thread, or NULL if the hash
@@ -182,6 +182,11 @@ FreeThreadStorageEntry(
* May change an entry in the master thread storage cache to point to the
* specified thread and it's associated hash table.
*
+ * Thread safety:
+ * This function assumes that integer operations are safe (atomic)
+ * on all (currently) supported Tcl platforms. Hence there are
+ * places where shared integer arithmetic is done w/o protective locks.
+ *
*----------------------------------------------------------------------
*/
@@ -194,11 +199,31 @@ ThreadStorageGetHashTable(
int isNew;
Tcl_HashTable *hashTablePtr;
- Tcl_MutexLock(&threadStorageLock);
+ /*
+ * It's important that we pick up the hash table pointer BEFORE comparing
+ * thread Id in case another thread is in the critical region changing
+ * things out from under you.
+ *
+ * Thread safety: threadStorageCache is accessed w/o locks in order to
+ * avoid serialization of all threads at this hot-spot. It is safe to
+ * do this here because (threadStorageCache[index].id != id) test below
+ * should be atomic on all (currently) supported platforms and there
+ * are no devastatig side effects of the test.
+ *
+ * Note Valgrind users: this place will show up as a race-condition in
+ * helgrind-tool output. To silence this warnings, define VALGRIND
+ * symbol at compilation time.
+ */
+#if !defined(VALGRIND)
hashTablePtr = threadStorageCache[index].hashTablePtr;
-
if (threadStorageCache[index].id != id) {
+ Tcl_MutexLock(&threadStorageLock);
+#else
+ Tcl_MutexLock(&threadStorageLock);
+ hashTablePtr = threadStorageCache[index].hashTablePtr;
+ if (threadStorageCache[index].id != id) {
+#endif
/*
* It's not in the cache, so we look it up...
@@ -252,9 +277,13 @@ ThreadStorageGetHashTable(
threadStorageCache[index].id = id;
threadStorageCache[index].hashTablePtr = hashTablePtr;
+#if !defined(VALGRIND)
+ Tcl_MutexUnlock(&threadStorageLock);
+ }
+#else
}
-
Tcl_MutexUnlock(&threadStorageLock);
+#endif
return hashTablePtr;
}