- Revision
- 126208
- Author
- [email protected]
- Date
- 2012-08-21 16:16:24 -0700 (Tue, 21 Aug 2012)
Log Message
WTF Threading leaks kernel objects on platforms that use pthreads
https://bugs.webkit.org/show_bug.cgi?id=94636
Reviewed by Geoffrey Garen.
Source/WTF:
Creating lots of Web workers on Mac platforms leaks lots of Mach ports. Eventually, the
process can exhaust its allocation of Mach ports from the kernel, which can then cause
all sorts of badness, including the inability to allocate new virtual memory from the
kernel. ThreadingPthreads.cpp and ThreadIdentifierDataPthreads.cpp need to be refactored
so that we do not leak these kernel resources. I would assume that we also leak kernel
resources on other pthreads platforms as well.
* wtf/ThreadIdentifierDataPthreads.cpp:
(WTF):
(WTF::ThreadIdentifierData::~ThreadIdentifierData): Now calls the event threadDidExit, which
handles all relevant tear-down of the thread metadata in the thread map.
* wtf/ThreadingPthreads.cpp: Added a new class called PthreadState that encapsulates the
state of a thread and the possible transitions between those states.
(PthreadState):
(WTF::PthreadState::PthreadState):
(WTF::PthreadState::joinableState): Returns the current state of the pthread.
(WTF::PthreadState::pthreadHandle): Returns the pthread_t for this particular thread. This needs to
remain valid even after the thread has exited because somebody could come along at any time in the
future and call join on the thread.
(WTF::PthreadState::didBecomeDetached): Signals that the thread was detached.
(WTF::PthreadState::didExit): Signals that the thread's exit destructor (~ThreadIdentifierData) has run.
(WTF::PthreadState::didJoin): Signals that the thread has been joined on successfully.
(WTF::PthreadState::hasExited): Returns whether or not the thread's exit destructor has run.
(WTF):
(WTF::identifierByPthreadHandle): Changed to also check hasExited() on the threads it finds in the map. We
should only have one valid pthread_t in the map, but there are other pthread_t's that need to remain in the
thread map for when somebody joins on that thread id later.
(WTF::establishIdentifierForPthreadHandle): Changed to put the allocate the new PthreadState data structure and
put it in the map.
(WTF::pthreadHandleForIdentifierWithLockAlreadyHeld):
(WTF::wtfThreadEntryPoint):
(WTF::waitForThreadCompletion): We now do the relevant cleanup after the specified thread has been
successfully joined on depending on if the joined thread has already exited.
(WTF::detachThread): Performs relevant cleanup if the thread has already exited. Otherwise signals to the
PthreadState that the thread has been detached.
(WTF::threadDidExit): Function called by ~ThreadIdentifierData to indicate that the thread has exited.
Only cleans up after itself if the thread isn't Joinable (i.e. Joined or Detached).
LayoutTests:
Added a test that creates a bunch of workers that immediately return. This should stress
the new WTF threading code on platforms that use pthreads, so any major regressions in correctness
will probably cause this test to fail since it creates both joinable and detached threads.
* fast/js/create-lots-of-workers-expected.txt: Added.
* fast/js/create-lots-of-workers.html: Added.
* fast/js/resources/empty-worker.js: Added.
Modified Paths
Added Paths
Diff
Modified: trunk/LayoutTests/ChangeLog (126207 => 126208)
--- trunk/LayoutTests/ChangeLog 2012-08-21 23:14:23 UTC (rev 126207)
+++ trunk/LayoutTests/ChangeLog 2012-08-21 23:16:24 UTC (rev 126208)
@@ -1,3 +1,18 @@
+2012-08-21 Mark Hahnenberg <[email protected]>
+
+ WTF Threading leaks kernel objects on platforms that use pthreads
+ https://bugs.webkit.org/show_bug.cgi?id=94636
+
+ Reviewed by Geoffrey Garen.
+
+ Added a test that creates a bunch of workers that immediately return. This should stress
+ the new WTF threading code on platforms that use pthreads, so any major regressions in correctness
+ will probably cause this test to fail since it creates both joinable and detached threads.
+
+ * fast/js/create-lots-of-workers-expected.txt: Added.
+ * fast/js/create-lots-of-workers.html: Added.
+ * fast/js/resources/empty-worker.js: Added.
+
2012-08-21 Florin Malita <[email protected]>
ASSERT triggered in SVGTRefTargetEventListener::handleEvent()
Added: trunk/LayoutTests/fast/js/create-lots-of-workers-expected.txt (0 => 126208)
--- trunk/LayoutTests/fast/js/create-lots-of-workers-expected.txt (rev 0)
+++ trunk/LayoutTests/fast/js/create-lots-of-workers-expected.txt 2012-08-21 23:16:24 UTC (rev 126208)
@@ -0,0 +1,2 @@
+PASSED
+
Added: trunk/LayoutTests/fast/js/create-lots-of-workers.html (0 => 126208)
--- trunk/LayoutTests/fast/js/create-lots-of-workers.html (rev 0)
+++ trunk/LayoutTests/fast/js/create-lots-of-workers.html 2012-08-21 23:16:24 UTC (rev 126208)
@@ -0,0 +1,31 @@
+<!DOCTYPE HTML>
+<html>
+<head>
+<title>Web Worker Test</title>
+<script src=""
+</head>
+<body>
+<script>
+window.jsTestIsAsync = true;
+var iters = 0;
+
+var startLotsOfEmptyWorkers = function() {
+ for (var i = 0; i < 100; i++) {
+ var w = new Worker('resources/empty-worker.js');
+ }
+ if (iters < 2) {
+ iters += 1;
+ setTimeout(startLotsOfEmptyWorkers, 1000);
+ } else if (testRunner) {
+ setTimeout(function() {
+ debug("PASSED");
+ testRunner.notifyDone();
+ }, 1000);
+ }
+};
+
+startLotsOfEmptyWorkers();
+</script>
+<script src=""
+</body>
+</html>
Added: trunk/LayoutTests/fast/js/resources/empty-worker.js (0 => 126208)
--- trunk/LayoutTests/fast/js/resources/empty-worker.js (rev 0)
+++ trunk/LayoutTests/fast/js/resources/empty-worker.js 2012-08-21 23:16:24 UTC (rev 126208)
@@ -0,0 +1 @@
+self.close();
Modified: trunk/Source/WTF/ChangeLog (126207 => 126208)
--- trunk/Source/WTF/ChangeLog 2012-08-21 23:14:23 UTC (rev 126207)
+++ trunk/Source/WTF/ChangeLog 2012-08-21 23:16:24 UTC (rev 126208)
@@ -1,3 +1,48 @@
+2012-08-21 Mark Hahnenberg <[email protected]>
+
+ WTF Threading leaks kernel objects on platforms that use pthreads
+ https://bugs.webkit.org/show_bug.cgi?id=94636
+
+ Reviewed by Geoffrey Garen.
+
+ Creating lots of Web workers on Mac platforms leaks lots of Mach ports. Eventually, the
+ process can exhaust its allocation of Mach ports from the kernel, which can then cause
+ all sorts of badness, including the inability to allocate new virtual memory from the
+ kernel. ThreadingPthreads.cpp and ThreadIdentifierDataPthreads.cpp need to be refactored
+ so that we do not leak these kernel resources. I would assume that we also leak kernel
+ resources on other pthreads platforms as well.
+
+ * wtf/ThreadIdentifierDataPthreads.cpp:
+ (WTF):
+ (WTF::ThreadIdentifierData::~ThreadIdentifierData): Now calls the event threadDidExit, which
+ handles all relevant tear-down of the thread metadata in the thread map.
+ * wtf/ThreadingPthreads.cpp: Added a new class called PthreadState that encapsulates the
+ state of a thread and the possible transitions between those states.
+ (PthreadState):
+ (WTF::PthreadState::PthreadState):
+ (WTF::PthreadState::joinableState): Returns the current state of the pthread.
+ (WTF::PthreadState::pthreadHandle): Returns the pthread_t for this particular thread. This needs to
+ remain valid even after the thread has exited because somebody could come along at any time in the
+ future and call join on the thread.
+ (WTF::PthreadState::didBecomeDetached): Signals that the thread was detached.
+ (WTF::PthreadState::didExit): Signals that the thread's exit destructor (~ThreadIdentifierData) has run.
+ (WTF::PthreadState::didJoin): Signals that the thread has been joined on successfully.
+ (WTF::PthreadState::hasExited): Returns whether or not the thread's exit destructor has run.
+ (WTF):
+ (WTF::identifierByPthreadHandle): Changed to also check hasExited() on the threads it finds in the map. We
+ should only have one valid pthread_t in the map, but there are other pthread_t's that need to remain in the
+ thread map for when somebody joins on that thread id later.
+ (WTF::establishIdentifierForPthreadHandle): Changed to put the allocate the new PthreadState data structure and
+ put it in the map.
+ (WTF::pthreadHandleForIdentifierWithLockAlreadyHeld):
+ (WTF::wtfThreadEntryPoint):
+ (WTF::waitForThreadCompletion): We now do the relevant cleanup after the specified thread has been
+ successfully joined on depending on if the joined thread has already exited.
+ (WTF::detachThread): Performs relevant cleanup if the thread has already exited. Otherwise signals to the
+ PthreadState that the thread has been detached.
+ (WTF::threadDidExit): Function called by ~ThreadIdentifierData to indicate that the thread has exited.
+ Only cleans up after itself if the thread isn't Joinable (i.e. Joined or Detached).
+
2012-08-21 Ulan Degenbaev <[email protected]>
Call AdjustAmountOfExternalAllocatedMemory when V8ArrayBuffer constructed and destructed
Modified: trunk/Source/WTF/wtf/ThreadIdentifierDataPthreads.cpp (126207 => 126208)
--- trunk/Source/WTF/wtf/ThreadIdentifierDataPthreads.cpp 2012-08-21 23:14:23 UTC (rev 126207)
+++ trunk/Source/WTF/wtf/ThreadIdentifierDataPthreads.cpp 2012-08-21 23:16:24 UTC (rev 126208)
@@ -47,11 +47,11 @@
pthread_key_t ThreadIdentifierData::m_key = PTHREAD_KEYS_MAX;
-void clearPthreadHandleForIdentifier(ThreadIdentifier);
+void threadDidExit(ThreadIdentifier);
ThreadIdentifierData::~ThreadIdentifierData()
{
- clearPthreadHandleForIdentifier(m_identifier);
+ threadDidExit(m_identifier);
}
void ThreadIdentifierData::initializeOnce()
Modified: trunk/Source/WTF/wtf/ThreadingPthreads.cpp (126207 => 126208)
--- trunk/Source/WTF/wtf/ThreadingPthreads.cpp 2012-08-21 23:14:23 UTC (rev 126207)
+++ trunk/Source/WTF/wtf/ThreadingPthreads.cpp 2012-08-21 23:16:24 UTC (rev 126208)
@@ -61,11 +61,47 @@
namespace WTF {
-typedef HashMap<ThreadIdentifier, pthread_t> ThreadMap;
+class PthreadState {
+public:
+ enum JoinableState {
+ Joinable, // The default thread state. The thread can be joined on.
+ Joined, // Somebody waited on this thread to exit and this thread finally exited. This state is here because there can be a
+ // period of time between when the thread exits (which causes pthread_join to return and the remainder of waitOnThreadCompletion to run)
+ // and when threadDidExit is called. We need threadDidExit to take charge and delete the thread data since there's
+ // nobody else to pick up the slack in this case (since waitOnThreadCompletion has already returned).
+
+ Detached // The thread has been detached and can no longer be joined on. At this point, the thread must take care of cleaning up after itself.
+ };
+
+ // Currently all threads created by WTF start out as joinable.
+ PthreadState(pthread_t handle)
+ : m_joinableState(Joinable)
+ , m_didExit(false)
+ , m_pthreadHandle(handle)
+ {
+ }
+
+ JoinableState joinableState() { return m_joinableState; }
+ pthread_t pthreadHandle() { return m_pthreadHandle; }
+ void didBecomeDetached() { m_joinableState = Detached; }
+ void didExit() { m_didExit = true; }
+ void didJoin() { m_joinableState = Joined; }
+ bool hasExited() { return m_didExit; }
+
+private:
+ JoinableState m_joinableState;
+ bool m_didExit;
+ pthread_t m_pthreadHandle;
+};
+
+typedef HashMap<ThreadIdentifier, OwnPtr<PthreadState> > ThreadMap;
+
static Mutex* atomicallyInitializedStaticMutex;
-void clearPthreadHandleForIdentifier(ThreadIdentifier);
+void unsafeThreadWasDetached(ThreadIdentifier);
+void threadDidExit(ThreadIdentifier);
+void threadWasJoined(ThreadIdentifier);
static Mutex& threadMapMutex()
{
@@ -114,7 +150,7 @@
ThreadMap::iterator i = threadMap().begin();
for (; i != threadMap().end(); ++i) {
- if (pthread_equal(i->second, pthreadHandle))
+ if (pthread_equal(i->second->pthreadHandle(), pthreadHandle) && !i->second->hasExited())
return i->first;
}
@@ -124,38 +160,22 @@
static ThreadIdentifier establishIdentifierForPthreadHandle(const pthread_t& pthreadHandle)
{
ASSERT(!identifierByPthreadHandle(pthreadHandle));
-
MutexLocker locker(threadMapMutex());
-
static ThreadIdentifier identifierCount = 1;
-
- threadMap().add(identifierCount, pthreadHandle);
-
+ threadMap().add(identifierCount, adoptPtr(new PthreadState(pthreadHandle)));
return identifierCount++;
}
-static pthread_t pthreadHandleForIdentifier(ThreadIdentifier id)
+static pthread_t pthreadHandleForIdentifierWithLockAlreadyHeld(ThreadIdentifier id)
{
- MutexLocker locker(threadMapMutex());
-
- return threadMap().get(id);
+ return threadMap().get(id)->pthreadHandle();
}
-void clearPthreadHandleForIdentifier(ThreadIdentifier id)
-{
- MutexLocker locker(threadMapMutex());
-
- ASSERT(threadMap().contains(id));
-
- threadMap().remove(id);
-}
-
static void* wtfThreadEntryPoint(void* param)
{
// Balanced by .leakPtr() in createThreadInternal.
OwnPtr<ThreadFunctionInvocation> invocation = adoptPtr(static_cast<ThreadFunctionInvocation*>(param));
invocation->function(invocation->data);
-
return 0;
}
@@ -198,16 +218,35 @@
int waitForThreadCompletion(ThreadIdentifier threadID)
{
+ pthread_t pthreadHandle;
ASSERT(threadID);
- pthread_t pthreadHandle = pthreadHandleForIdentifier(threadID);
- if (!pthreadHandle)
- return 0;
+ {
+ // We don't want to lock across the call to join, since that can block our thread and cause deadlock.
+ MutexLocker locker(threadMapMutex());
+ pthreadHandle = pthreadHandleForIdentifierWithLockAlreadyHeld(threadID);
+ ASSERT(pthreadHandle);
+ }
int joinResult = pthread_join(pthreadHandle, 0);
+
if (joinResult == EDEADLK)
LOG_ERROR("ThreadIdentifier %u was found to be deadlocked trying to quit", threadID);
+ else if (joinResult)
+ LOG_ERROR("ThreadIdentifier %u was unable to be joined.\n", threadID);
+ MutexLocker locker(threadMapMutex());
+ PthreadState* state = threadMap().get(threadID);
+ ASSERT(state);
+ ASSERT(state->joinableState() == PthreadState::Joinable);
+
+ // The thread has already exited, so clean up after it.
+ if (state->hasExited())
+ threadMap().remove(threadID);
+ // The thread hasn't exited yet, so don't clean anything up. Just signal that we've already joined on it so that it will clean up after itself.
+ else
+ state->didJoin();
+
return joinResult;
}
@@ -215,13 +254,34 @@
{
ASSERT(threadID);
- pthread_t pthreadHandle = pthreadHandleForIdentifier(threadID);
- if (!pthreadHandle)
- return;
+ MutexLocker locker(threadMapMutex());
+ pthread_t pthreadHandle = pthreadHandleForIdentifierWithLockAlreadyHeld(threadID);
+ ASSERT(pthreadHandle);
- pthread_detach(pthreadHandle);
+ int detachResult = pthread_detach(pthreadHandle);
+ if (detachResult)
+ LOG_ERROR("ThreadIdentifier %u was unable to be detached\n", threadID);
+
+ PthreadState* state = threadMap().get(threadID);
+ ASSERT(state);
+ if (state->hasExited())
+ threadMap().remove(threadID);
+ else
+ threadMap().get(threadID)->didBecomeDetached();
}
+void threadDidExit(ThreadIdentifier threadID)
+{
+ MutexLocker locker(threadMapMutex());
+ PthreadState* state = threadMap().get(threadID);
+ ASSERT(state);
+
+ state->didExit();
+
+ if (state->joinableState() != PthreadState::Joinable)
+ threadMap().remove(threadID);
+}
+
void yield()
{
sched_yield();
