[SCM] WebKit Debian packaging branch, webkit-1.1, updated. upstream/1.1.19-706-ge5415e9
dimich at chromium.org
dimich at chromium.org
Thu Feb 4 21:25:10 UTC 2010
The following commit has been merged in the webkit-1.1 branch:
commit 331b484623b7f920121179db7d9dc1fae2bac896
Author: dimich at chromium.org <dimich at chromium.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Date: Fri Jan 22 22:00:37 2010 +0000
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):
git-svn-id: http://svn.webkit.org/repository/webkit/trunk@53714 268f45cc-cd09-0410-ab3c-d52691b4dbfc
diff --git a/JavaScriptCore/Android.mk b/JavaScriptCore/Android.mk
index 1dd5ec0..e76da32 100644
--- a/JavaScriptCore/Android.mk
+++ b/JavaScriptCore/Android.mk
@@ -159,6 +159,7 @@ LOCAL_SRC_FILES := \
wtf/RandomNumber.cpp \
wtf/RefCountedLeakCounter.cpp \
wtf/TCSystemAlloc.cpp \
+ wtf/ThreadIdentifierDataPthreads.cpp \
wtf/Threading.cpp \
wtf/ThreadingPthreads.cpp \
\
diff --git a/JavaScriptCore/Android.v8.wtf.mk b/JavaScriptCore/Android.v8.wtf.mk
index 53a50d9..69128d6 100644
--- a/JavaScriptCore/Android.v8.wtf.mk
+++ b/JavaScriptCore/Android.v8.wtf.mk
@@ -42,6 +42,7 @@ LOCAL_SRC_FILES := \
wtf/RandomNumber.cpp \
wtf/RefCountedLeakCounter.cpp \
wtf/TCSystemAlloc.cpp \
+ wtf/ThreadIdentifierDataPthreads.cpp \
wtf/Threading.cpp \
wtf/ThreadingPthreads.cpp \
\
diff --git a/JavaScriptCore/ChangeLog b/JavaScriptCore/ChangeLog
index 57e615f..13d2e45 100644
--- a/JavaScriptCore/ChangeLog
+++ b/JavaScriptCore/ChangeLog
@@ -1,3 +1,53 @@
+2009-01-22 Dmitry Titov <dimich at chromium.org>
+
+ Reviewed by Maciej Stachowiak.
+
+ Fix the leak of ThreadIdentifiers in threadMap across threads.
+ https://bugs.webkit.org/show_bug.cgi?id=32689
+
+ 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.
+
2010-01-21 Kwang Yul Seo <skyul at company100.net>
Reviewed by Alexey Proskuryakov.
diff --git a/JavaScriptCore/GNUmakefile.am b/JavaScriptCore/GNUmakefile.am
index 13d6092..7ac6a8c 100644
--- a/JavaScriptCore/GNUmakefile.am
+++ b/JavaScriptCore/GNUmakefile.am
@@ -279,10 +279,12 @@ javascriptcore_sources += \
JavaScriptCore/wtf/TCPackedCache.h \
JavaScriptCore/wtf/TCPageMap.h \
JavaScriptCore/wtf/TCSpinLock.h \
- JavaScriptCore/wtf/ThreadSpecific.h \
+ JavaScriptCore/wtf/ThreadIdentifierDataPthreads.cpp \
+ JavaScriptCore/wtf/ThreadIdentifierDataPthreads.h \
JavaScriptCore/wtf/Threading.cpp \
JavaScriptCore/wtf/Threading.h \
JavaScriptCore/wtf/ThreadingPthreads.cpp \
+ JavaScriptCore/wtf/ThreadSpecific.h \
JavaScriptCore/wtf/TypeTraits.cpp \
JavaScriptCore/wtf/TypeTraits.h \
JavaScriptCore/wtf/UnusedParam.h \
diff --git a/JavaScriptCore/JavaScriptCore.gyp/JavaScriptCore.gyp b/JavaScriptCore/JavaScriptCore.gyp/JavaScriptCore.gyp
index 64e20a2..66f40e9 100644
--- a/JavaScriptCore/JavaScriptCore.gyp/JavaScriptCore.gyp
+++ b/JavaScriptCore/JavaScriptCore.gyp/JavaScriptCore.gyp
@@ -134,6 +134,7 @@
'conditions': [
['OS=="win"', {
'sources/': [
+ ['exclude', 'ThreadIdentifierDataPthreads\\.(h|cpp)$'],
['exclude', 'ThreadingPthreads\\.cpp$'],
['include', 'Thread(ing|Specific)Win\\.cpp$']
],
diff --git a/JavaScriptCore/JavaScriptCore.gypi b/JavaScriptCore/JavaScriptCore.gypi
index c31cfce..24577da 100644
--- a/JavaScriptCore/JavaScriptCore.gypi
+++ b/JavaScriptCore/JavaScriptCore.gypi
@@ -418,6 +418,8 @@
'wtf/TCSpinLock.h',
'wtf/TCSystemAlloc.cpp',
'wtf/TCSystemAlloc.h',
+ 'wtf/ThreadIdentifierDataPthreads.cpp',
+ 'wtf/ThreadIdentifierDataPthreads.h',
'wtf/Threading.cpp',
'wtf/Threading.h',
'wtf/ThreadingNone.cpp',
diff --git a/JavaScriptCore/JavaScriptCore.xcodeproj/project.pbxproj b/JavaScriptCore/JavaScriptCore.xcodeproj/project.pbxproj
index a114866..ade1be1 100644
--- a/JavaScriptCore/JavaScriptCore.xcodeproj/project.pbxproj
+++ b/JavaScriptCore/JavaScriptCore.xcodeproj/project.pbxproj
@@ -177,6 +177,8 @@
14F8BA4F107EC899009892DC /* Collector.cpp in Sources */ = {isa = PBXBuildFile; fileRef = F692A8520255597D01FF60F7 /* Collector.cpp */; };
180B9B080F16D94F009BDBC5 /* CurrentTime.h in Headers */ = {isa = PBXBuildFile; fileRef = 180B9AF00F16C569009BDBC5 /* CurrentTime.h */; settings = {ATTRIBUTES = (Private, ); }; };
180B9BFE0F16E94D009BDBC5 /* CurrentTime.cpp in Sources */ = {isa = PBXBuildFile; fileRef = 180B9AEF0F16C569009BDBC5 /* CurrentTime.cpp */; };
+ 18BAB55310DAE054000D945B /* ThreadIdentifierDataPthreads.cpp in Sources */ = {isa = PBXBuildFile; fileRef = 18BAB52710DADFCD000D945B /* ThreadIdentifierDataPthreads.cpp */; };
+ 18BAB55410DAE066000D945B /* ThreadIdentifierDataPthreads.h in Headers */ = {isa = PBXBuildFile; fileRef = 18BAB52810DADFCD000D945B /* ThreadIdentifierDataPthreads.h */; };
1C61516C0EBAC7A00031376F /* ProfilerServer.mm in Sources */ = {isa = PBXBuildFile; fileRef = 1C61516A0EBAC7A00031376F /* ProfilerServer.mm */; settings = {COMPILER_FLAGS = "-fno-strict-aliasing"; }; };
1C61516D0EBAC7A00031376F /* ProfilerServer.h in Headers */ = {isa = PBXBuildFile; fileRef = 1C61516B0EBAC7A00031376F /* ProfilerServer.h */; };
41359CF30FDD89AD00206180 /* DateConversion.h in Headers */ = {isa = PBXBuildFile; fileRef = D21202290AD4310C00ED79B6 /* DateConversion.h */; };
@@ -654,6 +656,8 @@
14F3488E0E95EF8A003648BC /* CollectorHeapIterator.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = CollectorHeapIterator.h; sourceTree = "<group>"; };
180B9AEF0F16C569009BDBC5 /* CurrentTime.cpp */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; path = CurrentTime.cpp; sourceTree = "<group>"; };
180B9AF00F16C569009BDBC5 /* CurrentTime.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = CurrentTime.h; sourceTree = "<group>"; };
+ 18BAB52710DADFCD000D945B /* ThreadIdentifierDataPthreads.cpp */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; path = ThreadIdentifierDataPthreads.cpp; sourceTree = "<group>"; };
+ 18BAB52810DADFCD000D945B /* ThreadIdentifierDataPthreads.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = ThreadIdentifierDataPthreads.h; sourceTree = "<group>"; };
1C61516A0EBAC7A00031376F /* ProfilerServer.mm */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.objcpp; name = ProfilerServer.mm; path = profiler/ProfilerServer.mm; sourceTree = "<group>"; };
1C61516B0EBAC7A00031376F /* ProfilerServer.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; name = ProfilerServer.h; path = profiler/ProfilerServer.h; sourceTree = "<group>"; };
1C9051420BA9E8A70081E9D0 /* Version.xcconfig */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = text.xcconfig; path = Version.xcconfig; sourceTree = "<group>"; };
@@ -1371,6 +1375,8 @@
6541BD6F08E80A17002CBEE7 /* TCSpinLock.h */,
6541BD7008E80A17002CBEE7 /* TCSystemAlloc.cpp */,
6541BD7108E80A17002CBEE7 /* TCSystemAlloc.h */,
+ 18BAB52710DADFCD000D945B /* ThreadIdentifierDataPthreads.cpp */,
+ 18BAB52810DADFCD000D945B /* ThreadIdentifierDataPthreads.h */,
5D6A566A0F05995500266145 /* Threading.cpp */,
E1EE79220D6C95CD00FEA3BA /* Threading.h */,
E1EE793C0D6C9B9200FEA3BA /* ThreadingPthreads.cpp */,
@@ -2038,6 +2044,7 @@
148CD1D8108CF902008163C6 /* JSContextRefPrivate.h in Headers */,
14A1563210966365006FA260 /* DateInstanceCache.h in Headers */,
1420BE7B10AA6DDB00F455D2 /* WeakRandom.h in Headers */,
+ 18BAB55410DAE066000D945B /* ThreadIdentifierDataPthreads.h in Headers */,
8698B86910D44D9400D8D01B /* StringBuilder.h in Headers */,
8698BB3910D86BAF00D8D01B /* UStringImpl.h in Headers */,
14035DB110DBFB2A00FFFFE7 /* WeakGCPtr.h in Headers */,
@@ -2475,6 +2482,7 @@
7E4EE70F0EBB7A5B005934AA /* StructureChain.cpp in Sources */,
BCCF0D0C0EF0B8A500413C8F /* StructureStubInfo.cpp in Sources */,
14F8BA43107EC88C009892DC /* TCSystemAlloc.cpp in Sources */,
+ 18BAB55310DAE054000D945B /* ThreadIdentifierDataPthreads.cpp in Sources */,
5D6A566B0F05995500266145 /* Threading.cpp in Sources */,
E1EE793D0D6C9B9200FEA3BA /* ThreadingPthreads.cpp in Sources */,
14A42E3F0F4F60EE00599099 /* TimeoutChecker.cpp in Sources */,
diff --git a/JavaScriptCore/wtf/ThreadIdentifierDataPthreads.cpp b/JavaScriptCore/wtf/ThreadIdentifierDataPthreads.cpp
new file mode 100644
index 0000000..042d49e
--- /dev/null
+++ b/JavaScriptCore/wtf/ThreadIdentifierDataPthreads.cpp
@@ -0,0 +1,97 @@
+/*
+ * Copyright (C) 2009 Google Inc. All rights reserved.
+ *
+ * Redistribution and use in source and binary forms, with or without
+ * modification, are permitted provided that the following conditions are
+ * met:
+ *
+ * * Redistributions of source code must retain the above copyright
+ * notice, this list of conditions and the following disclaimer.
+ * * Redistributions in binary form must reproduce the above
+ * copyright notice, this list of conditions and the following disclaimer
+ * in the documentation and/or other materials provided with the
+ * distribution.
+ * * Neither the name of Google Inc. nor the names of its
+ * contributors may be used to endorse or promote products derived from
+ * this software without specific prior written permission.
+ *
+ * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
+ * "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
+ * LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
+ * A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT
+ * OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
+ * SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
+ * LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
+ * DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
+ * THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
+ * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
+ * OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+ */
+
+#include "config.h"
+
+#if USE(PTHREADS)
+
+#include "ThreadIdentifierDataPthreads.h"
+
+#include "Threading.h"
+
+namespace WTF {
+
+pthread_key_t ThreadIdentifierData::m_key;
+
+void clearPthreadHandleForIdentifier(ThreadIdentifier);
+
+ThreadIdentifierData::~ThreadIdentifierData()
+{
+ clearPthreadHandleForIdentifier(m_identifier);
+}
+
+ThreadIdentifier ThreadIdentifierData::identifier()
+{
+ initializeKeyOnce();
+ ThreadIdentifierData* threadIdentifierData = static_cast<ThreadIdentifierData*>(pthread_getspecific(m_key));
+
+ return threadIdentifierData ? threadIdentifierData->m_identifier : 0;
+}
+
+void ThreadIdentifierData::initialize(ThreadIdentifier id)
+{
+ ASSERT(!identifier());
+
+ initializeKeyOnce();
+ pthread_setspecific(m_key, new ThreadIdentifierData(id));
+}
+
+void ThreadIdentifierData::destruct(void* data)
+{
+ ThreadIdentifierData* threadIdentifierData = static_cast<ThreadIdentifierData*>(data);
+ ASSERT(threadIdentifierData);
+
+ if (threadIdentifierData->m_isDestroyedOnce) {
+ delete threadIdentifierData;
+ return;
+ }
+
+ threadIdentifierData->m_isDestroyedOnce = true;
+ // Re-setting the value for key causes another destruct() call after all other thread-specific destructors were called.
+ pthread_setspecific(m_key, threadIdentifierData);
+}
+
+void ThreadIdentifierData::initializeKeyOnceHelper()
+{
+ if (pthread_key_create(&m_key, destruct))
+ CRASH();
+}
+
+void ThreadIdentifierData::initializeKeyOnce()
+{
+ static pthread_once_t onceControl = PTHREAD_ONCE_INIT;
+ if (pthread_once(&onceControl, initializeKeyOnceHelper))
+ CRASH();
+}
+
+} // namespace WTF
+
+#endif // USE(PTHREADS)
+
diff --git a/JavaScriptCore/wtf/ThreadIdentifierDataPthreads.h b/JavaScriptCore/wtf/ThreadIdentifierDataPthreads.h
new file mode 100644
index 0000000..3af87a8
--- /dev/null
+++ b/JavaScriptCore/wtf/ThreadIdentifierDataPthreads.h
@@ -0,0 +1,77 @@
+/*
+ * Copyright (C) 2009 Google Inc. All rights reserved.
+ *
+ * Redistribution and use in source and binary forms, with or without
+ * modification, are permitted provided that the following conditions are
+ * met:
+ *
+ * * Redistributions of source code must retain the above copyright
+ * notice, this list of conditions and the following disclaimer.
+ * * Redistributions in binary form must reproduce the above
+ * copyright notice, this list of conditions and the following disclaimer
+ * in the documentation and/or other materials provided with the
+ * distribution.
+ * * Neither the name of Google Inc. nor the names of its
+ * contributors may be used to endorse or promote products derived from
+ * this software without specific prior written permission.
+ *
+ * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
+ * "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
+ * LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
+ * A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT
+ * OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
+ * SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
+ * LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
+ * DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
+ * THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
+ * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
+ * OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+ */
+
+#ifndef ThreadIdentifierDataPthreads_h
+#define ThreadIdentifierDataPthreads_h
+
+#include <wtf/Noncopyable.h>
+#include <wtf/Threading.h>
+
+namespace WTF {
+
+// Holds ThreadIdentifier in the thread-specific storage and employs pthreads-specific 2-pass destruction to reliably remove
+// ThreadIdentifier from threadMap. It assumes regular ThreadSpecific types don't use multiple-pass destruction.
+class ThreadIdentifierData : public Noncopyable {
+public:
+ ~ThreadIdentifierData();
+
+ // Creates and puts an instance of ThreadIdentifierData into thread-specific storage.
+ static void initialize(ThreadIdentifier identifier);
+
+ // Returns 0 if thread-specific storage was not initialized.
+ static ThreadIdentifier identifier();
+
+private:
+ ThreadIdentifierData(ThreadIdentifier identifier)
+ : m_identifier(identifier)
+ , m_isDestroyedOnce(false)
+ {
+ }
+
+ // This thread-specific destructor is called 2 times when thread terminates:
+ // - first, when all the other thread-specific destructors are called, it simply remembers it was 'destroyed once'
+ // and re-sets itself into the thread-specific slot to make Pthreads to call it again later.
+ // - second, after all thread-specific destructors were invoked, it gets called again - this time, we remove the
+ // ThreadIdentifier from the threadMap, completing the cleanup.
+ static void destruct(void* data);
+
+ static void initializeKeyOnceHelper();
+ static void initializeKeyOnce();
+
+ ThreadIdentifier m_identifier;
+ bool m_isDestroyedOnce;
+ static pthread_key_t m_key;
+};
+
+} // namespace WTF
+
+#endif // ThreadIdentifierDataPthreads_h
+
+
diff --git a/JavaScriptCore/wtf/Threading.cpp b/JavaScriptCore/wtf/Threading.cpp
index 74c47f4..49de59e 100644
--- a/JavaScriptCore/wtf/Threading.cpp
+++ b/JavaScriptCore/wtf/Threading.cpp
@@ -49,13 +49,14 @@ static void* threadEntryPoint(void* contextData)
{
NewThreadContext* context = reinterpret_cast<NewThreadContext*>(contextData);
- setThreadNameInternal(context->name);
-
- // Block until our creating thread has completed any extra setup work.
+ // Block until our creating thread has completed any extra setup work, including
+ // establishing ThreadIdentifier.
{
MutexLocker locker(context->creationMutex);
}
+ initializeCurrentThreadInternal(context->name);
+
// Grab the info that we need out of the context, then deallocate it.
ThreadFunction entryPoint = context->entryPoint;
void* data = context->data;
diff --git a/JavaScriptCore/wtf/Threading.h b/JavaScriptCore/wtf/Threading.h
index 3a6ae59..1599562 100644
--- a/JavaScriptCore/wtf/Threading.h
+++ b/JavaScriptCore/wtf/Threading.h
@@ -121,7 +121,7 @@ ThreadIdentifier createThreadInternal(ThreadFunction, void*, const char* threadN
// Called in the thread during initialization.
// Helpful for platforms where the thread name must be set from within the thread.
-void setThreadNameInternal(const char* threadName);
+void initializeCurrentThreadInternal(const char* threadName);
ThreadIdentifier currentThread();
bool isMainThread();
diff --git a/JavaScriptCore/wtf/ThreadingNone.cpp b/JavaScriptCore/wtf/ThreadingNone.cpp
index 63a0ada..2e8a259 100644
--- a/JavaScriptCore/wtf/ThreadingNone.cpp
+++ b/JavaScriptCore/wtf/ThreadingNone.cpp
@@ -36,7 +36,7 @@ namespace WTF {
void initializeThreading() { }
ThreadIdentifier createThreadInternal(ThreadFunction, void*, const char*) { return ThreadIdentifier(); }
-void setThreadNameInternal(const char*) { }
+void initializeCurrentThreadInternal(const char*) { }
int waitForThreadCompletion(ThreadIdentifier, void**) { return 0; }
void detachThread(ThreadIdentifier) { }
ThreadIdentifier currentThread() { return ThreadIdentifier(); }
diff --git a/JavaScriptCore/wtf/ThreadingPthreads.cpp b/JavaScriptCore/wtf/ThreadingPthreads.cpp
index 9f6ff7c..d610d5f 100644
--- a/JavaScriptCore/wtf/ThreadingPthreads.cpp
+++ b/JavaScriptCore/wtf/ThreadingPthreads.cpp
@@ -37,6 +37,8 @@
#include "MainThread.h"
#include "RandomNumberSeed.h"
#include "StdLibExtras.h"
+#include "ThreadIdentifierDataPthreads.h"
+#include "ThreadSpecific.h"
#include "UnusedParam.h"
#include <errno.h>
@@ -108,7 +110,7 @@ static ThreadIdentifier identifierByPthreadHandle(const pthread_t& pthreadHandle
return 0;
}
-static ThreadIdentifier establishIdentifierForPthreadHandle(pthread_t& pthreadHandle)
+static ThreadIdentifier establishIdentifierForPthreadHandle(const pthread_t& pthreadHandle)
{
ASSERT(!identifierByPthreadHandle(pthreadHandle));
@@ -128,7 +130,7 @@ static pthread_t pthreadHandleForIdentifier(ThreadIdentifier id)
return threadMap().get(id);
}
-static void clearPthreadHandleForIdentifier(ThreadIdentifier id)
+void clearPthreadHandleForIdentifier(ThreadIdentifier id)
{
MutexLocker locker(threadMapMutex());
@@ -185,13 +187,17 @@ ThreadIdentifier createThreadInternal(ThreadFunction entryPoint, void* data, con
}
#endif
-void setThreadNameInternal(const char* threadName)
+void initializeCurrentThreadInternal(const char* threadName)
{
#if HAVE(PTHREAD_SETNAME_NP)
pthread_setname_np(threadName);
#else
UNUSED_PARAM(threadName);
#endif
+
+ ThreadIdentifier id = identifierByPthreadHandle(pthread_self());
+ ASSERT(id);
+ ThreadIdentifierData::initialize(id);
}
int waitForThreadCompletion(ThreadIdentifier threadID, void** result)
@@ -204,7 +210,6 @@ int waitForThreadCompletion(ThreadIdentifier threadID, void** result)
if (joinResult == EDEADLK)
LOG_ERROR("ThreadIdentifier %u was found to be deadlocked trying to quit", threadID);
- clearPthreadHandleForIdentifier(threadID);
return joinResult;
}
@@ -215,16 +220,18 @@ void detachThread(ThreadIdentifier threadID)
pthread_t pthreadHandle = pthreadHandleForIdentifier(threadID);
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;
}
bool isMainThread()
diff --git a/JavaScriptCore/wtf/ThreadingWin.cpp b/JavaScriptCore/wtf/ThreadingWin.cpp
index 1fd84d2..73c3f0c 100644
--- a/JavaScriptCore/wtf/ThreadingWin.cpp
+++ b/JavaScriptCore/wtf/ThreadingWin.cpp
@@ -118,7 +118,7 @@ typedef struct tagTHREADNAME_INFO {
} THREADNAME_INFO;
#pragma pack(pop)
-void setThreadNameInternal(const char* szThreadName)
+void initializeCurrentThreadInternal(const char* szThreadName)
{
THREADNAME_INFO info;
info.dwType = 0x1000;
@@ -161,7 +161,7 @@ void initializeThreading()
initializeRandomNumberGenerator();
initializeMainThread();
mainThreadIdentifier = currentThread();
- setThreadNameInternal("Main Thread");
+ initializeCurrentThreadInternal("Main Thread");
}
}
diff --git a/JavaScriptCore/wtf/gtk/ThreadingGtk.cpp b/JavaScriptCore/wtf/gtk/ThreadingGtk.cpp
index b4f4de1..a6ec8f7 100644
--- a/JavaScriptCore/wtf/gtk/ThreadingGtk.cpp
+++ b/JavaScriptCore/wtf/gtk/ThreadingGtk.cpp
@@ -138,7 +138,7 @@ ThreadIdentifier createThreadInternal(ThreadFunction entryPoint, void* data, con
return threadID;
}
-void setThreadNameInternal(const char*)
+void initializeCurrentThreadInternal(const char*)
{
}
diff --git a/JavaScriptCore/wtf/qt/ThreadingQt.cpp b/JavaScriptCore/wtf/qt/ThreadingQt.cpp
index 9bd400c..dc04a68 100644
--- a/JavaScriptCore/wtf/qt/ThreadingQt.cpp
+++ b/JavaScriptCore/wtf/qt/ThreadingQt.cpp
@@ -182,7 +182,7 @@ ThreadIdentifier createThreadInternal(ThreadFunction entryPoint, void* data, con
return establishIdentifierForThread(threadRef);
}
-void setThreadNameInternal(const char*)
+void initializeCurrentThreadInternal(const char*)
{
}
diff --git a/WebKitTools/ChangeLog b/WebKitTools/ChangeLog
index 18ade1a..3aaa51f 100644
--- a/WebKitTools/ChangeLog
+++ b/WebKitTools/ChangeLog
@@ -1,3 +1,24 @@
+2010-01-22 Dmitry Titov <dimich at chromium.org>
+
+ Reviewed by Maciej Stachowiak.
+
+ Fix the leak of ThreadIdentifiers in threadMap across threads.
+ https://bugs.webkit.org/show_bug.cgi?id=32689
+
+ 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):
+
2010-01-22 Kent Tamura <tkent at chromium.org>
Reviewed by Maciej Stachowiak.
diff --git a/WebKitTools/DumpRenderTree/mac/DumpRenderTree.mm b/WebKitTools/DumpRenderTree/mac/DumpRenderTree.mm
index 12f216e..12e1941 100644
--- a/WebKitTools/DumpRenderTree/mac/DumpRenderTree.mm
+++ b/WebKitTools/DumpRenderTree/mac/DumpRenderTree.mm
@@ -80,6 +80,7 @@
#import <objc/objc-runtime.h>
#import <wtf/Assertions.h>
#import <wtf/RetainPtr.h>
+#import <wtf/Threading.h>
#import <wtf/OwnPtr.h>
using namespace std;
@@ -466,6 +467,31 @@ static void setDefaultsToConsistentValuesForTesting()
}
+static void* runThread(void* arg)
+{
+ static ThreadIdentifier previousId = 0;
+ ThreadIdentifier currentId = currentThread();
+ // Verify 2 successive threads do not get the same Id.
+ ASSERT(previousId != currentId);
+ previousId = currentId;
+ return 0;
+}
+
+static void testThreadIdentifierMap()
+{
+ // Imitate 'foreign' threads that are not created by WTF.
+ pthread_t pthread;
+ pthread_create(&pthread, 0, &runThread, 0);
+ pthread_join(pthread, 0);
+
+ pthread_create(&pthread, 0, &runThread, 0);
+ pthread_join(pthread, 0);
+
+ // Now create another thread using WTF. On OSX, it will have the same pthread handle
+ // but should get a different ThreadIdentifier.
+ createThread(runThread, 0, "DumpRenderTree: test");
+}
+
static void crashHandler(int sig)
{
char *signalName = strsignal(sig);
@@ -614,6 +640,9 @@ void dumpRenderTree(int argc, const char *argv[])
// <rdar://problem/5222911>
testStringByEvaluatingJavaScriptFromString();
+ // http://webkit.org/b/32689
+ testThreadIdentifierMap();
+
if (threaded)
startJavaScriptThreads();
--
WebKit Debian packaging
More information about the Pkg-webkit-commits
mailing list