Changeset 53714 in webkit for trunk/JavaScriptCore/wtf/ThreadingPthreads.cpp

Timestamp:

Jan 22, 2010, 2:00:37 PM (15 years ago)

Author:

[email protected]

Message:

Fix the leak of ThreadIdentifiers in threadMap across threads.
​https://bugs.webkit.org/show_bug.cgi?id=32689

Reviewed by Maciej Stachowiak.

JavaScriptCore:

Test is added to DumpRenderTree.mm.

• Android.mk: Added file ThreadIdentifierDataPthreads.(h|cpp) to build.
• Android.v8.wtf.mk: Ditto.
• GNUmakefile.am: Ditto.
• JavaScriptCore.gyp/JavaScriptCore.gyp: Ditto.
• JavaScriptCore.gypi: Ditto.
• JavaScriptCore.xcodeproj/project.pbxproj: Ditto.

• wtf/ThreadIdentifierDataPthreads.cpp: Added. Contains custom implementation of thread-specific data that uses custom destructor.

(WTF::ThreadIdentifierData::~ThreadIdentifierData): Removes the ThreadIdentifier from the threadMap.
(WTF::ThreadIdentifierData::identifier):
(WTF::ThreadIdentifierData::initialize):
(WTF::ThreadIdentifierData::destruct): Custom thread-specific destructor. Resets the value for the key again to cause second invoke.
(WTF::ThreadIdentifierData::initializeKeyOnceHelper):
(WTF::ThreadIdentifierData::initializeKeyOnce): Need to use pthread_once since initialization may come on any thread(s).

• wtf/ThreadIdentifierDataPthreads.h: Added.

(WTF::ThreadIdentifierData::ThreadIdentifierData):

• wtf/Threading.cpp:

(WTF::threadEntryPoint): Move initializeCurrentThreadInternal to after the lock to make

sure it is invoked when ThreadIdentifier is already established.

• wtf/Threading.h: Rename setThreadNameInternal -> initializeCurrentThreadInternal since it does more then only set the name now.
• wtf/ThreadingNone.cpp:

(WTF::initializeCurrentThreadInternal): Ditto.

• wtf/ThreadingWin.cpp:

(WTF::initializeCurrentThreadInternal): Ditto.
(WTF::initializeThreading): Ditto.

• wtf/gtk/ThreadingGtk.cpp:

(WTF::initializeCurrentThreadInternal): Ditto.

• wtf/qt/ThreadingQt.cpp:

(WTF::initializeCurrentThreadInternal): Ditto.

• wtf/ThreadingPthreads.cpp:

(WTF::establishIdentifierForPthreadHandle):
(WTF::clearPthreadHandleForIdentifier): Make it not 'static' so the ~ThreadIdentifierData() in another file can call it.
(WTF::initializeCurrentThreadInternal): Set the thread-specific data. The ThreadIdentifier is already established by creating thread.
(WTF::waitForThreadCompletion): Remove call to clearPthreadHandleForIdentifier(threadID) since it is now done in ~ThreadIdentifierData().
(WTF::detachThread): Ditto.
(WTF::currentThread): Use the thread-specific data to get the ThreadIdentifier. It's many times faster then Mutex-protected iteration through the map.

Also, set the thread-specific data if called first time on the thread.

WebKitTools:

Add a new test to verify the ThreadIdentifiers are not reused across threads.
The test runs in the beginning of DumpRenderTree and spawns 2 non-WTF treads sequentially,
waiting for the previous thread to terminate before starting the next.
The treads use WTF::currentThread() in their thread function. Without a fix, this
causes both threads to have the same ThreadIdentifier which triggers ASSERT in thread function.
It also starts another thread using WTF. Without the fix, this finds pthread handle from previous
threads in the WTF threadMap and asserts in WTF::establishIdentifierForPthreadHandle().
The test practically does not affect the DRT run time because the threads end immediately.

• DumpRenderTree/mac/DumpRenderTree.mm:

(runThread): Test thread function.
(testThreadIdentifierMap):
(dumpRenderTree):

File:

• trunk/JavaScriptCore/wtf/ThreadingPthreads.cpp (modified) (7 diffs)

trunk/JavaScriptCore/wtf/ThreadingPthreads.cpp

@@ -38 +38 @@
 #include "RandomNumberSeed.h"
 #include "StdLibExtras.h"
+#include "ThreadIdentifierDataPthreads.h"
+#include "ThreadSpecific.h"
 #include "UnusedParam.h"
 #include <errno.h>
@@ -109 +111 @@
 }
 
-static ThreadIdentifier establishIdentifierForPthreadHandle(pthread_t& pthreadHandle)
+static ThreadIdentifier establishIdentifierForPthreadHandle(const pthread_t& pthreadHandle)
 {
     ASSERT(!identifierByPthreadHandle(pthreadHandle));
@@ -129 +131 @@
 }
 
-static void clearPthreadHandleForIdentifier(ThreadIdentifier id)
+void clearPthreadHandleForIdentifier(ThreadIdentifier id)
 {
     MutexLocker locker(threadMapMutex());
@@ -186 +188 @@
 #endif
 
-void setThreadNameInternal(const char* threadName)
+void initializeCurrentThreadInternal(const char* threadName)
 {
 #if HAVE(PTHREAD_SETNAME_NP)
@@ -193 +195 @@
     UNUSED_PARAM(threadName);
 #endif
+
+    ThreadIdentifier id = identifierByPthreadHandle(pthread_self());
+    ASSERT(id);
+    ThreadIdentifierData::initialize(id);
 }
 
@@ -205 +211 @@
         LOG_ERROR("ThreadIdentifier %u was found to be deadlocked trying to quit", threadID);
 
-    clearPthreadHandleForIdentifier(threadID);
     return joinResult;
 }
@@ -216 +221 @@
 
     pthread_detach(pthreadHandle);
-
-    clearPthreadHandleForIdentifier(threadID);
 }
 
 ThreadIdentifier currentThread()
 {
-    pthread_t currentThread = pthread_self();
-    if (ThreadIdentifier id = identifierByPthreadHandle(currentThread))
+    ThreadIdentifier id = ThreadIdentifierData::identifier();
+    if (id)
         return id;
-    return establishIdentifierForPthreadHandle(currentThread);
+
+    // Not a WTF-created thread, ThreadIdentifier is not established yet.
+    id = establishIdentifierForPthreadHandle(pthread_self());
+    ThreadIdentifierData::initialize(id);
+    return id;
 }