[cherry-pick] WTF Threading leaks kernel objects on platforms that use pthreads
authormhahnenberg@apple.com <mhahnenberg@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 21 Aug 2012 23:16:24 +0000 (23:16 +0000)
committerGerrit Code Review <gerrit@gerrit.vlan144.tizendev.org>
Tue, 25 Jun 2013 06:23:07 +0000 (06:23 +0000)
[Title] WTF Threading leaks kernel objects on platforms that use pthreads
[Issues] DCM-2005
[Problem] Layout is broken while flicking under on DynamicBox icon of PhotoViewer
[Solution] Routines of destorying pthreads fixed not to make memory leaked
[Cherry-Picker] Hojong Han <hojong.han@samsung.com>

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.

git-svn-id: http://svn.webkit.org/repository/webkit/trunk@126208 268f45cc-cd09-0410-ab3c-d52691b4dbfc

Conflicts:

LayoutTests/ChangeLog
Source/WTF/ChangeLog

Change-Id: I287841829550ce4940c280cbc38cecc5d2bce981

Conflicts:

LayoutTests/ChangeLog

LayoutTests/ChangeLog
LayoutTests/fast/js/create-lots-of-workers-expected.txt [new file with mode: 0644]
LayoutTests/fast/js/create-lots-of-workers.html [new file with mode: 0644]
LayoutTests/fast/js/resources/empty-worker.js [new file with mode: 0644]
Source/WTF/ChangeLog
Source/WTF/wtf/ThreadIdentifierDataPthreads.cpp
Source/WTF/wtf/ThreadingPthreads.cpp

index af1c3ea..492b02b 100644 (file)
@@ -1,3 +1,18 @@
+2012-08-21  Mark Hahnenberg  <mhahnenberg@apple.com>
+
+        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.
+
 2013-02-01 Nayan Kumar K <nayankk@motorola.com>
 
         [WEBGL] Rename WEBKIT_WEBGL_lose_context to WEBGL_lose_context.
diff --git a/LayoutTests/fast/js/create-lots-of-workers-expected.txt b/LayoutTests/fast/js/create-lots-of-workers-expected.txt
new file mode 100644 (file)
index 0000000..ef05cc8
--- /dev/null
@@ -0,0 +1,2 @@
+PASSED
+
diff --git a/LayoutTests/fast/js/create-lots-of-workers.html b/LayoutTests/fast/js/create-lots-of-workers.html
new file mode 100644 (file)
index 0000000..6964f5b
--- /dev/null
@@ -0,0 +1,31 @@
+<!DOCTYPE HTML>
+<html>
+<head>
+<title>Web Worker Test</title>
+<script src="resources/js-test-pre.js"></script>
+</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="resources/js-test-post.js"></script>
+</body>
+</html>
diff --git a/LayoutTests/fast/js/resources/empty-worker.js b/LayoutTests/fast/js/resources/empty-worker.js
new file mode 100644 (file)
index 0000000..8fd2734
--- /dev/null
@@ -0,0 +1 @@
+self.close();
index 0adb1fb..2405611 100644 (file)
 
         * wtf/Platform.h: Added ENABLE(COMPUTED_GOTO_OPCODES).
 
+2012-08-21  Mark Hahnenberg  <mhahnenberg@apple.com>
+
+        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-07-28  Patrick Gansterer  <paroga@webkit.org>
 
         [WIN] Add missing export macro to friend decleration.
index 0badf93..4ad20c8 100644 (file)
@@ -47,11 +47,11 @@ namespace WTF {
 
 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()
index 921dd3b..226ecbd 100644 (file)
 
 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 @@ static ThreadIdentifier identifierByPthreadHandle(const pthread_t& pthreadHandle
 
     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,30 +160,15 @@ static ThreadIdentifier identifierByPthreadHandle(const pthread_t& pthreadHandle
 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);
-}
-
-void clearPthreadHandleForIdentifier(ThreadIdentifier id)
-{
-    MutexLocker locker(threadMapMutex());
-
-    ASSERT(threadMap().contains(id));
-
-    threadMap().remove(id);
+    return threadMap().get(id)->pthreadHandle();
 }
 
 static void* wtfThreadEntryPoint(void* param)
@@ -155,7 +176,6 @@ 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,15 +218,34 @@ void initializeCurrentThreadInternal(const char* threadName)
 
 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,11 +254,32 @@ void detachThread(ThreadIdentifier threadID)
 {
     ASSERT(threadID);
 
-    pthread_t pthreadHandle = pthreadHandleForIdentifier(threadID);
-    if (!pthreadHandle)
-        return;
+    MutexLocker locker(threadMapMutex());
+    pthread_t pthreadHandle = pthreadHandleForIdentifierWithLockAlreadyHeld(threadID);
+    ASSERT(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();
 
-    pthread_detach(pthreadHandle);
+    if (state->joinableState() != PthreadState::Joinable)
+        threadMap().remove(threadID);
 }
 
 void yield()