[WTF] Use ThreadGroup to bookkeep active threads for Mach exception
authorutatane.tea@gmail.com <utatane.tea@gmail.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 20 Jul 2017 18:32:13 +0000 (18:32 +0000)
committerutatane.tea@gmail.com <utatane.tea@gmail.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 20 Jul 2017 18:32:13 +0000 (18:32 +0000)
https://bugs.webkit.org/show_bug.cgi?id=174678

Reviewed by Mark Lam.

Source/JavaScriptCore:

Use Thread& instead.

* runtime/JSLock.cpp:
(JSC::JSLock::didAcquireLock):

Source/WTF:

We can use ThreadGroup to bookkeep active threads for Mach exceptions.
When the thread dies, it is automatically removed from the thread groups.
So we do not need to call unregisterThreadForMachExceptionHandling.

* wtf/ThreadGroup.cpp:
(WTF::ThreadGroup::~ThreadGroup):
(WTF::ThreadGroup::add):
* wtf/ThreadGroup.h:
* wtf/ThreadHolder.cpp:
(WTF::ThreadHolder::~ThreadHolder):
* wtf/Threading.cpp:
(WTF::Thread::addToThreadGroup):
(WTF::Thread::removeFromThreadGroup):
* wtf/Threading.h:
* wtf/threads/Signals.cpp:
(WTF::setExceptionPorts):
(WTF::activeThreads):
(WTF::registerThreadForMachExceptionHandling):
(WTF::installSignalHandler):
(WTF::unregisterThreadForMachExceptionHandling): Deleted.
* wtf/threads/Signals.h:

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

Source/JavaScriptCore/ChangeLog
Source/JavaScriptCore/runtime/JSLock.cpp
Source/WTF/ChangeLog
Source/WTF/wtf/ThreadGroup.cpp
Source/WTF/wtf/ThreadGroup.h
Source/WTF/wtf/ThreadHolder.cpp
Source/WTF/wtf/Threading.cpp
Source/WTF/wtf/Threading.h
Source/WTF/wtf/threads/Signals.cpp
Source/WTF/wtf/threads/Signals.h

index fe8cca2..5df65a2 100644 (file)
@@ -1,3 +1,15 @@
+2017-07-20  Yusuke Suzuki  <utatane.tea@gmail.com>
+
+        [WTF] Use ThreadGroup to bookkeep active threads for Mach exception
+        https://bugs.webkit.org/show_bug.cgi?id=174678
+
+        Reviewed by Mark Lam.
+
+        Use Thread& instead.
+
+        * runtime/JSLock.cpp:
+        (JSC::JSLock::didAcquireLock):
+
 2017-07-19  Yusuke Suzuki  <utatane.tea@gmail.com>
 
         [WTF] Implement WTF::ThreadGroup
index f83512f..dbd6a58 100644 (file)
@@ -151,7 +151,7 @@ void JSLock::didAcquireLock()
 #endif
 
 #if HAVE(MACH_EXCEPTIONS)
-    registerThreadForMachExceptionHandling(&Thread::current());
+    registerThreadForMachExceptionHandling(Thread::current());
 #endif
 
     // Note: everything below must come after addCurrentThread().
index 65ff3d0..378bb61 100644 (file)
@@ -1,3 +1,32 @@
+2017-07-20  Yusuke Suzuki  <utatane.tea@gmail.com>
+
+        [WTF] Use ThreadGroup to bookkeep active threads for Mach exception
+        https://bugs.webkit.org/show_bug.cgi?id=174678
+
+        Reviewed by Mark Lam.
+
+        We can use ThreadGroup to bookkeep active threads for Mach exceptions.
+        When the thread dies, it is automatically removed from the thread groups.
+        So we do not need to call unregisterThreadForMachExceptionHandling.
+
+        * wtf/ThreadGroup.cpp:
+        (WTF::ThreadGroup::~ThreadGroup):
+        (WTF::ThreadGroup::add):
+        * wtf/ThreadGroup.h:
+        * wtf/ThreadHolder.cpp:
+        (WTF::ThreadHolder::~ThreadHolder):
+        * wtf/Threading.cpp:
+        (WTF::Thread::addToThreadGroup):
+        (WTF::Thread::removeFromThreadGroup):
+        * wtf/Threading.h:
+        * wtf/threads/Signals.cpp:
+        (WTF::setExceptionPorts):
+        (WTF::activeThreads):
+        (WTF::registerThreadForMachExceptionHandling):
+        (WTF::installSignalHandler):
+        (WTF::unregisterThreadForMachExceptionHandling): Deleted.
+        * wtf/threads/Signals.h:
+
 2017-07-19  Yusuke Suzuki  <utatane.tea@gmail.com>
 
         [WTF] Remove unnecessary indirection of WTF::Thread entry point
index 829c990..f4e2408 100644 (file)
@@ -32,17 +32,22 @@ namespace WTF {
 
 ThreadGroup::~ThreadGroup()
 {
-    std::lock_guard<std::mutex> locker(m_lock);
+    auto locker = holdLock(m_lock);
     for (auto& thread : m_threads)
         thread->removeFromThreadGroup(locker, *this);
 }
 
-bool ThreadGroup::add(Thread& thread)
+bool ThreadGroup::add(const AbstractLocker& locker, Thread& thread)
 {
-    std::lock_guard<std::mutex> locker(m_lock);
     return thread.addToThreadGroup(locker, *this);
 }
 
+bool ThreadGroup::add(Thread& thread)
+{
+    auto locker = holdLock(m_lock);
+    return add(locker, thread);
+}
+
 void ThreadGroup::addCurrentThread()
 {
     bool isAdded = add(Thread::current());
index db94900..1d9a67f 100644 (file)
@@ -42,6 +42,7 @@ public:
     }
 
     WTF_EXPORT_PRIVATE bool add(Thread&);
+    WTF_EXPORT_PRIVATE bool add(const AbstractLocker&, Thread&);
     WTF_EXPORT_PRIVATE void addCurrentThread();
 
     const ListHashSet<Ref<Thread>>& threads(const AbstractLocker&) const { return m_threads; }
index 977d1df..e2a31be 100644 (file)
 
 #include "Threading.h"
 
-#include <wtf/threads/Signals.h>
-
 namespace WTF {
 
 ThreadSpecificKey ThreadHolder::m_key = InvalidThreadSpecificKey;
 
 ThreadHolder::~ThreadHolder()
 {
-#if HAVE(MACH_EXCEPTIONS)
-    unregisterThreadForMachExceptionHandling(&m_thread.get());
-#endif
     m_thread->didExit();
 }
 
index 2c20258..db4462a 100644 (file)
@@ -173,7 +173,7 @@ void Thread::didExit()
     m_didExit = true;
 }
 
-bool Thread::addToThreadGroup(const std::lock_guard<std::mutex>& threadGroupLocker, ThreadGroup& threadGroup)
+bool Thread::addToThreadGroup(const AbstractLocker& threadGroupLocker, ThreadGroup& threadGroup)
 {
     UNUSED_PARAM(threadGroupLocker);
     std::lock_guard<std::mutex> locker(m_mutex);
@@ -184,7 +184,7 @@ bool Thread::addToThreadGroup(const std::lock_guard<std::mutex>& threadGroupLock
     return true;
 }
 
-void Thread::removeFromThreadGroup(const std::lock_guard<std::mutex>& threadGroupLocker, ThreadGroup& threadGroup)
+void Thread::removeFromThreadGroup(const AbstractLocker& threadGroupLocker, ThreadGroup& threadGroup)
 {
     UNUSED_PARAM(threadGroupLocker);
     std::lock_guard<std::mutex> locker(m_mutex);
index 5b48f0a..9e82a63 100644 (file)
@@ -182,8 +182,8 @@ protected:
     bool hasExited() { return m_didExit; }
 
     // These functions are only called from ThreadGroup.
-    bool addToThreadGroup(const std::lock_guard<std::mutex>& threadGroupLocker, ThreadGroup&);
-    void removeFromThreadGroup(const std::lock_guard<std::mutex>& threadGroupLocker, ThreadGroup&);
+    bool addToThreadGroup(const AbstractLocker& threadGroupLocker, ThreadGroup&);
+    void removeFromThreadGroup(const AbstractLocker& threadGroupLocker, ThreadGroup&);
 
     // WordLock & Lock rely on ThreadSpecific. But Thread object can be destroyed even after ThreadSpecific things are destroyed.
     std::mutex m_mutex;
index 56973a9..22f6b3a 100644 (file)
@@ -46,9 +46,9 @@ extern "C" {
 
 #include <wtf/Atomics.h>
 #include <wtf/DataLog.h>
-#include <wtf/HashSet.h>
 #include <wtf/LocklessBag.h>
 #include <wtf/NeverDestroyed.h>
+#include <wtf/ThreadGroup.h>
 #include <wtf/ThreadMessage.h>
 #include <wtf/Threading.h>
 
@@ -215,39 +215,31 @@ void handleSignalsWithMach()
 }
 
 
-static StaticLock threadLock;
 exception_mask_t activeExceptions { 0 };
 
-inline void setExceptionPorts(const AbstractLocker&, Thread* thread)
+inline void setExceptionPorts(const AbstractLocker& threadGroupLocker, Thread& thread)
 {
-    kern_return_t result = thread_set_exception_ports(thread->machThread(), activeExceptions, exceptionPort, EXCEPTION_STATE | MACH_EXCEPTION_CODES, MACHINE_THREAD_STATE);
+    UNUSED_PARAM(threadGroupLocker);
+    kern_return_t result = thread_set_exception_ports(thread.machThread(), activeExceptions, exceptionPort, EXCEPTION_STATE | MACH_EXCEPTION_CODES, MACHINE_THREAD_STATE);
     if (result != KERN_SUCCESS) {
         dataLogLn("thread set port failed due to ", mach_error_string(result));
         CRASH();
     }
 }
 
-inline HashSet<Thread*>& activeThreads(const AbstractLocker&)
+inline ThreadGroup& activeThreads()
 {
-    static NeverDestroyed<HashSet<Thread*>> activeThreads;
-    return activeThreads;
+    static NeverDestroyed<std::shared_ptr<ThreadGroup>> activeThreads { ThreadGroup::create() };
+    return *activeThreads.get();
 }
 
-void registerThreadForMachExceptionHandling(Thread* thread)
+void registerThreadForMachExceptionHandling(Thread& thread)
 {
-    auto locker = holdLock(threadLock);
-    auto result = activeThreads(locker).add(thread);
-
-    if (result.isNewEntry)
+    auto locker = holdLock(activeThreads().getLock());
+    if (activeThreads().add(locker, thread))
         setExceptionPorts(locker, thread);
 }
 
-void unregisterThreadForMachExceptionHandling(Thread* thread)
-{
-    auto locker = holdLock(threadLock);
-    activeThreads(locker).remove(thread);
-}
-
 #else
 static constexpr bool useMach = false;
 #endif // HAVE(MACH_EXCEPTIONS)
@@ -292,12 +284,12 @@ void installSignalHandler(Signal signal, SignalHandler&& handler)
     handlers[static_cast<size_t>(signal)]->add(WTFMove(handler));
 
 #if HAVE(MACH_EXCEPTIONS)
-    auto locker = holdLock(threadLock);
+    auto locker = holdLock(activeThreads().getLock());
     if (useMach) {
         activeExceptions |= toMachMask(signal);
 
-        for (Thread* thread : activeThreads(locker))
-            setExceptionPorts(locker, thread);
+        for (auto& thread : activeThreads().threads(locker))
+            setExceptionPorts(locker, thread.get());
     }
 #endif
 }
index faae0a7..9c8d10c 100644 (file)
@@ -91,8 +91,7 @@ WTF_EXPORT_PRIVATE void installSignalHandler(Signal, SignalHandler&&);
 
 #if HAVE(MACH_EXCEPTIONS)
 class Thread;
-void registerThreadForMachExceptionHandling(Thread*);
-void unregisterThreadForMachExceptionHandling(Thread*);
+void registerThreadForMachExceptionHandling(Thread&);
 
 void handleSignalsWithMach();
 #endif // HAVE(MACH_EXCEPTIONS)
@@ -101,7 +100,6 @@ void handleSignalsWithMach();
 
 #if HAVE(MACH_EXCEPTIONS)
 using WTF::registerThreadForMachExceptionHandling;
-using WTF::unregisterThreadForMachExceptionHandling;
 using WTF::handleSignalsWithMach;
 #endif // HAVE(MACH_EXCEPTIONS)