Use WordLock instead of std::mutex for Threading
authorutatane.tea@gmail.com <utatane.tea@gmail.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Mon, 30 Apr 2018 17:30:26 +0000 (17:30 +0000)
committerutatane.tea@gmail.com <utatane.tea@gmail.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Mon, 30 Apr 2018 17:30:26 +0000 (17:30 +0000)
https://bugs.webkit.org/show_bug.cgi?id=185121

Reviewed by Geoffrey Garen.

Source/bmalloc:

Add constexpr to explicitly describe that bmalloc::Mutex constructor is constexpr.

* bmalloc/Mutex.h:

Source/JavaScriptCore:

ThreadGroup starts using WordLock.

* heap/MachineStackMarker.h:
(JSC::MachineThreads::getLock):

Source/WTF:

Before r231151, WordLock depends on ThreadSpecific. It means that our Threading implementation
cannot use this lock since Threading primitives could touch these locks after ThreadSpecific
for that WordLock is destroyed.

Now WordLock is changed not to use ThreadSpecific. So it does not depend on our Threading
mechanism and our Threading can start using WordLock internally.

This patch changes WTF::Thread and WTF::ThreadGroup to use WordLock instead of std::mutex.

And add constexpr to explicitly describe that Lock, Condition, and WordLock constructors are constexpr.

* wtf/Condition.h:
* wtf/Lock.h:
* wtf/ThreadGroup.h:
(WTF::ThreadGroup::getLock):
* wtf/Threading.cpp:
(WTF::Thread::didExit):
(WTF::Thread::addToThreadGroup):
(WTF::Thread::removeFromThreadGroup):
* wtf/Threading.h:
* wtf/ThreadingPthreads.cpp:
(WTF::Thread::changePriority):
(WTF::Thread::waitForCompletion):
(WTF::Thread::detach):
(WTF::Thread::signal):
(WTF::Thread::establishPlatformSpecificHandle):
* wtf/ThreadingWin.cpp:
(WTF::Thread::changePriority):
(WTF::Thread::waitForCompletion):
(WTF::Thread::detach):
(WTF::Thread::establishPlatformSpecificHandle):
(WTF::Thread::initializeTLSKey):
(WTF::Thread::currentDying):
(WTF::Thread::get):
(WTF::Thread::initializeTLS):
(WTF::Thread::destructTLS):
(WTF::threadMapMutex): Deleted.
* wtf/WordLock.h:

Tools:

* TestWebKitAPI/Tests/WTF/Signals.cpp:
(TEST):

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

15 files changed:
Source/JavaScriptCore/ChangeLog
Source/JavaScriptCore/heap/MachineStackMarker.h
Source/WTF/ChangeLog
Source/WTF/wtf/Condition.h
Source/WTF/wtf/Lock.h
Source/WTF/wtf/ThreadGroup.h
Source/WTF/wtf/Threading.cpp
Source/WTF/wtf/Threading.h
Source/WTF/wtf/ThreadingPthreads.cpp
Source/WTF/wtf/ThreadingWin.cpp
Source/WTF/wtf/WordLock.h
Source/bmalloc/ChangeLog
Source/bmalloc/bmalloc/Mutex.h
Tools/ChangeLog
Tools/TestWebKitAPI/Tests/WTF/Signals.cpp

index 4d0f8b51c8cbf4852c01d20dc675d76451d2ab10..e429ca410adda6dad933f4dee41fac156f0f4fe1 100644 (file)
@@ -1,3 +1,15 @@
+2018-04-30  Yusuke Suzuki  <utatane.tea@gmail.com>
+
+        Use WordLock instead of std::mutex for Threading
+        https://bugs.webkit.org/show_bug.cgi?id=185121
+
+        Reviewed by Geoffrey Garen.
+
+        ThreadGroup starts using WordLock.
+
+        * heap/MachineStackMarker.h:
+        (JSC::MachineThreads::getLock):
+
 2018-04-29  Filip Pizlo  <fpizlo@apple.com>
 
         B3 should run tail duplication at the bitter end
index 1d794db1218fff8e227840c88868a78c7f556d32..5fe07c355cfae13fc0537b82c5d08947e0775890 100644 (file)
@@ -50,7 +50,7 @@ public:
     // Only needs to be called by clients that can use the same heap from multiple threads.
     void addCurrentThread() { m_threadGroup->addCurrentThread(); }
 
-    std::mutex& getLock() { return m_threadGroup->getLock(); }
+    WordLock& getLock() { return m_threadGroup->getLock(); }
     const ListHashSet<Ref<Thread>>& threads(const AbstractLocker& locker) const { return m_threadGroup->threads(locker); }
 
 private:
index d7130ca15f59522d849810bc0072f4040a72799b..99f2b8fe24902afd2e9095eb7f3e551eff373b61 100644 (file)
@@ -1,3 +1,49 @@
+2018-04-30  Yusuke Suzuki  <utatane.tea@gmail.com>
+
+        Use WordLock instead of std::mutex for Threading
+        https://bugs.webkit.org/show_bug.cgi?id=185121
+
+        Reviewed by Geoffrey Garen.
+
+        Before r231151, WordLock depends on ThreadSpecific. It means that our Threading implementation
+        cannot use this lock since Threading primitives could touch these locks after ThreadSpecific
+        for that WordLock is destroyed.
+
+        Now WordLock is changed not to use ThreadSpecific. So it does not depend on our Threading
+        mechanism and our Threading can start using WordLock internally.
+
+        This patch changes WTF::Thread and WTF::ThreadGroup to use WordLock instead of std::mutex.
+
+        And add constexpr to explicitly describe that Lock, Condition, and WordLock constructors are constexpr.
+
+        * wtf/Condition.h:
+        * wtf/Lock.h:
+        * wtf/ThreadGroup.h:
+        (WTF::ThreadGroup::getLock):
+        * wtf/Threading.cpp:
+        (WTF::Thread::didExit):
+        (WTF::Thread::addToThreadGroup):
+        (WTF::Thread::removeFromThreadGroup):
+        * wtf/Threading.h:
+        * wtf/ThreadingPthreads.cpp:
+        (WTF::Thread::changePriority):
+        (WTF::Thread::waitForCompletion):
+        (WTF::Thread::detach):
+        (WTF::Thread::signal):
+        (WTF::Thread::establishPlatformSpecificHandle):
+        * wtf/ThreadingWin.cpp:
+        (WTF::Thread::changePriority):
+        (WTF::Thread::waitForCompletion):
+        (WTF::Thread::detach):
+        (WTF::Thread::establishPlatformSpecificHandle):
+        (WTF::Thread::initializeTLSKey):
+        (WTF::Thread::currentDying):
+        (WTF::Thread::get):
+        (WTF::Thread::initializeTLS):
+        (WTF::Thread::destructTLS):
+        (WTF::threadMapMutex): Deleted.
+        * wtf/WordLock.h:
+
 2018-04-29  Michael Catanzaro  <mcatanzaro@igalia.com>
 
         [CMake] Require GCC 6
index 512f10f2ad53caeba8079d606f676cc9fc2c0edc..75c8ea5d91288a3b56d77ed3497aa2c39df104d8 100644 (file)
@@ -48,7 +48,7 @@ public:
     // are unlikely to be affected by the cost of conversions, it is better to use MonotonicTime.
     using Time = ParkingLot::Time;
 
-    Condition() = default;
+    constexpr Condition() = default;
 
     // Wait on a parking queue while releasing the given lock. It will unlock the lock just before
     // parking, and relock it upon wakeup. Returns true if we woke up due to some call to
index d3c72152c1c4d8187d004c2bb82a1027d24fa873..e39470e1e5d295a4521c7a0d12a17e54f1f78437 100644 (file)
@@ -52,7 +52,7 @@ class Lock {
     WTF_MAKE_NONCOPYABLE(Lock);
     WTF_MAKE_FAST_ALLOCATED;
 public:
-    Lock() = default;
+    constexpr Lock() = default;
 
     void lock()
     {
index 88e419a111260774fd32cb180cb2778891a63aef..f53ce3196c2bdd6062a1b2adc66ef79b10cecf9a 100644 (file)
@@ -51,7 +51,7 @@ public:
 
     const ListHashSet<Ref<Thread>>& threads(const AbstractLocker&) const { return m_threads; }
 
-    std::mutex& getLock() { return m_lock; }
+    WordLock& getLock() { return m_lock; }
 
     WTF_EXPORT_PRIVATE ~ThreadGroup();
 
@@ -63,8 +63,8 @@ private:
         return shared_from_this();
     }
 
-    // We use std::mutex since it can be used when deallocating TLS.
-    std::mutex m_lock;
+    // We use WordLock since it can be used when deallocating TLS.
+    WordLock m_lock;
     ListHashSet<Ref<Thread>> m_threads;
 };
 
index ee8ad94c4805d0a7346101587f8f6fac760ff1cc..d4ce4fc5264ee5c29bea6ad161c0398ee480663a 100644 (file)
@@ -180,7 +180,7 @@ void Thread::didExit()
     if (shouldRemoveThreadFromThreadGroup()) {
         Vector<std::shared_ptr<ThreadGroup>> threadGroups;
         {
-            std::lock_guard<std::mutex> locker(m_mutex);
+            auto locker = holdLock(m_mutex);
             for (auto& threadGroup : m_threadGroups) {
                 // If ThreadGroup is just being destroyed,
                 // we do not need to perform unregistering.
@@ -190,8 +190,8 @@ void Thread::didExit()
             m_isShuttingDown = true;
         }
         for (auto& threadGroup : threadGroups) {
-            std::lock_guard<std::mutex> threadGroupLocker(threadGroup->getLock());
-            std::lock_guard<std::mutex> locker(m_mutex);
+            auto threadGroupLocker = holdLock(threadGroup->getLock());
+            auto locker = holdLock(m_mutex);
             threadGroup->m_threads.remove(*this);
         }
     }
@@ -200,14 +200,14 @@ void Thread::didExit()
 
     // We would like to say "thread is exited" after unregistering threads from thread groups.
     // So we need to separate m_isShuttingDown from m_didExit.
-    std::lock_guard<std::mutex> locker(m_mutex);
+    auto locker = holdLock(m_mutex);
     m_didExit = true;
 }
 
 ThreadGroupAddResult Thread::addToThreadGroup(const AbstractLocker& threadGroupLocker, ThreadGroup& threadGroup)
 {
     UNUSED_PARAM(threadGroupLocker);
-    std::lock_guard<std::mutex> locker(m_mutex);
+    auto locker = holdLock(m_mutex);
     if (m_isShuttingDown)
         return ThreadGroupAddResult::NotAdded;
     if (threadGroup.m_threads.add(*this).isNewEntry) {
@@ -220,7 +220,7 @@ ThreadGroupAddResult Thread::addToThreadGroup(const AbstractLocker& threadGroupL
 void Thread::removeFromThreadGroup(const AbstractLocker& threadGroupLocker, ThreadGroup& threadGroup)
 {
     UNUSED_PARAM(threadGroupLocker);
-    std::lock_guard<std::mutex> locker(m_mutex);
+    auto locker = holdLock(m_mutex);
     if (m_isShuttingDown)
         return;
     m_threadGroups.removeFirstMatching([&] (auto weakPtr) {
index 97cbe5418b9cfba9b326ced471ac81fbe1ad4725..47e019fb95c88d454fc1b54f914d633f57b64417 100644 (file)
@@ -45,6 +45,7 @@
 #include <wtf/ThreadSafeRefCounted.h>
 #include <wtf/ThreadSpecific.h>
 #include <wtf/Vector.h>
+#include <wtf/WordLock.h>
 
 #if USE(PTHREADS) && !OS(DARWIN)
 #include <signal.h>
@@ -273,8 +274,9 @@ protected:
     bool m_didExit { false };
     bool m_isDestroyedOnce { false };
 
-    // WordLock & Lock rely on ThreadSpecific. But Thread object can be destroyed even after ThreadSpecific things are destroyed.
-    std::mutex m_mutex;
+    // Lock & ParkingLot rely on ThreadSpecific. But Thread object can be destroyed even after ThreadSpecific things are destroyed.
+    // Use WordLock since WordLock does not depend on ThreadSpecific and this "Thread".
+    WordLock m_mutex;
     StackBounds m_stack { StackBounds::emptyBounds() };
     Vector<std::weak_ptr<ThreadGroup>> m_threadGroups;
     PlatformThreadHandle m_handle;
index ddac7374a3c6f9525cee62063ff59eff1761787f..80b1212581c0692d5e5a462ad49363280683a4f5 100644 (file)
@@ -256,7 +256,7 @@ void Thread::initializeCurrentThreadInternal(const char* threadName)
 
 void Thread::changePriority(int delta)
 {
-    std::lock_guard<std::mutex> locker(m_mutex);
+    auto locker = holdLock(m_mutex);
 
     int policy;
     struct sched_param param;
@@ -273,7 +273,7 @@ int Thread::waitForCompletion()
 {
     pthread_t handle;
     {
-        std::lock_guard<std::mutex> locker(m_mutex);
+        auto locker = holdLock(m_mutex);
         handle = m_handle;
     }
 
@@ -284,7 +284,7 @@ int Thread::waitForCompletion()
     else if (joinResult)
         LOG_ERROR("Thread %p was unable to be joined.\n", this);
 
-    std::lock_guard<std::mutex> locker(m_mutex);
+    auto locker = holdLock(m_mutex);
     ASSERT(joinableState() == Joinable);
 
     // If the thread has already exited, then do nothing. If the thread hasn't exited yet, then just signal that we've already joined on it.
@@ -297,7 +297,7 @@ int Thread::waitForCompletion()
 
 void Thread::detach()
 {
-    std::lock_guard<std::mutex> locker(m_mutex);
+    auto locker = holdLock(m_mutex);
     int detachResult = pthread_detach(m_handle);
     if (detachResult)
         LOG_ERROR("Thread %p was unable to be detached\n", this);
@@ -319,7 +319,7 @@ Thread& Thread::initializeCurrentTLS()
 
 bool Thread::signal(int signalNumber)
 {
-    std::lock_guard<std::mutex> locker(m_mutex);
+    auto locker = holdLock(m_mutex);
     if (hasExited())
         return false;
     int errNo = pthread_kill(m_handle, signalNumber);
@@ -440,7 +440,7 @@ size_t Thread::getRegisters(PlatformRegisters& registers)
 
 void Thread::establishPlatformSpecificHandle(pthread_t handle)
 {
-    std::lock_guard<std::mutex> locker(m_mutex);
+    auto locker = holdLock(m_mutex);
     m_handle = handle;
 #if OS(DARWIN)
     m_platformThread = pthread_mach_thread_np(handle);
index 6f3d41d6a0457fab0c06e8ed1b9d95c38c505c20..4e65a5e4277e4419e806652979695315998a56ee 100644 (file)
@@ -170,7 +170,7 @@ bool Thread::establishHandle(NewThreadContext* data)
 
 void Thread::changePriority(int delta)
 {
-    std::lock_guard<std::mutex> locker(m_mutex);
+    auto locker = holdLock(m_mutex);
     SetThreadPriority(m_handle, THREAD_PRIORITY_NORMAL + delta);
 }
 
@@ -178,7 +178,7 @@ int Thread::waitForCompletion()
 {
     HANDLE handle;
     {
-        std::lock_guard<std::mutex> locker(m_mutex);
+        auto locker = holdLock(m_mutex);
         handle = m_handle;
     }
 
@@ -186,7 +186,7 @@ int Thread::waitForCompletion()
     if (joinResult == WAIT_FAILED)
         LOG_ERROR("Thread %p was found to be deadlocked trying to quit", this);
 
-    std::lock_guard<std::mutex> locker(m_mutex);
+    auto locker = holdLock(m_mutex);
     ASSERT(joinableState() == Joinable);
 
     // The thread has already exited, do nothing.
@@ -208,7 +208,7 @@ void Thread::detach()
     // FlsCallback automatically. FlsCallback will call CloseHandle to clean up
     // resource. So in this function, we just mark the thread as detached to
     // avoid calling waitForCompletion for this thread.
-    std::lock_guard<std::mutex> locker(m_mutex);
+    auto locker = holdLock(m_mutex);
     if (!hasExited())
         didBecomeDetached();
 }
@@ -261,18 +261,14 @@ ThreadIdentifier Thread::currentID()
 
 void Thread::establishPlatformSpecificHandle(HANDLE handle, ThreadIdentifier threadID)
 {
-    std::lock_guard<std::mutex> locker(m_mutex);
+    auto locker = holdLock(m_mutex);
     m_handle = handle;
     m_id = threadID;
 }
 
 #define InvalidThread reinterpret_cast<Thread*>(static_cast<uintptr_t>(0xbbadbeef))
 
-static std::mutex& threadMapMutex()
-{
-    static NeverDestroyed<std::mutex> mutex;
-    return mutex.get();
-}
+static WordLock threadMapMutex;
 
 static HashMap<ThreadIdentifier, Thread*>& threadMap()
 {
@@ -282,7 +278,6 @@ static HashMap<ThreadIdentifier, Thread*>& threadMap()
 
 void Thread::initializeTLSKey()
 {
-    threadMapMutex();
     threadMap();
     threadSpecificKeyCreate(&s_key, destructTLS);
 }
@@ -291,14 +286,14 @@ Thread* Thread::currentDying()
 {
     ASSERT(s_key != InvalidThreadSpecificKey);
     // After FLS is destroyed, this map offers the value until the second thread exit callback is called.
-    std::lock_guard<std::mutex> locker(threadMapMutex());
+    auto locker = holdLock(threadMapMutex);
     return threadMap().get(currentID());
 }
 
 // FIXME: Remove this workaround code once <rdar://problem/31793213> is fixed.
 RefPtr<Thread> Thread::get(ThreadIdentifier id)
 {
-    std::lock_guard<std::mutex> locker(threadMapMutex());
+    auto locker = holdLock(threadMapMutex);
     Thread* thread = threadMap().get(id);
     if (thread)
         return thread;
@@ -314,7 +309,7 @@ Thread& Thread::initializeTLS(Ref<Thread>&& thread)
     auto& threadInTLS = thread.leakRef();
     threadSpecificSet(s_key, &threadInTLS);
     {
-        std::lock_guard<std::mutex> locker(threadMapMutex());
+        auto locker = holdLock(threadMapMutex);
         threadMap().add(id, &threadInTLS);
     }
     return threadInTLS;
@@ -348,7 +343,7 @@ void Thread::destructTLS(void* data)
 
     if (thread->m_isDestroyedOnce) {
         {
-            std::lock_guard<std::mutex> locker(threadMapMutex());
+            auto locker = holdLock(threadMapMutex);
             ASSERT(threadMap().contains(thread->id()));
             threadMap().remove(thread->id());
         }
index bf1354a6b86ce72d6ac7be1430a9359cf2343e24..8a90c1e81aa9f6463242e56dc20827af1ef45638 100644 (file)
@@ -50,7 +50,7 @@ class WordLock {
     WTF_MAKE_NONCOPYABLE(WordLock);
     WTF_MAKE_FAST_ALLOCATED;
 public:
-    WordLock() = default;
+    constexpr WordLock() = default;
 
     void lock()
     {
index c3787cb871c9d19037d5bc9f34e0d1725300fcb3..7afb578d27d90909e08550595621f70cb5748064 100644 (file)
@@ -1,3 +1,14 @@
+2018-04-30  Yusuke Suzuki  <utatane.tea@gmail.com>
+
+        Use WordLock instead of std::mutex for Threading
+        https://bugs.webkit.org/show_bug.cgi?id=185121
+
+        Reviewed by Geoffrey Garen.
+
+        Add constexpr to explicitly describe that bmalloc::Mutex constructor is constexpr.
+
+        * bmalloc/Mutex.h:
+
 2018-04-23  Ting-Wei Lan  <lantw44@gmail.com>
 
         Include stdio.h before using stderr
index a8c5ee6b02a672a3e7b448d7ef2a9a4a4ff1fc31..e18de8408fb9d66020966bc68cc744052b68b738 100644 (file)
@@ -37,7 +37,7 @@ namespace bmalloc {
 
 class Mutex {
 public:
-    Mutex() = default;
+    constexpr Mutex() = default;
 
     void lock();
     bool try_lock();
index efa7fe4c4dfce8bd274d0bbe0daa921468b5859f..394b4f83478db8b81ddc0524f56bac7eba9c12a1 100644 (file)
@@ -1,3 +1,13 @@
+2018-04-30  Yusuke Suzuki  <utatane.tea@gmail.com>
+
+        Use WordLock instead of std::mutex for Threading
+        https://bugs.webkit.org/show_bug.cgi?id=185121
+
+        Reviewed by Geoffrey Garen.
+
+        * TestWebKitAPI/Tests/WTF/Signals.cpp:
+        (TEST):
+
 2018-04-30  Michael Catanzaro  <mcatanzaro@igalia.com>
 
         [GTK] Webkit should spoof as Safari on a Mac when on Chase.com
index 4b9e834029db70f0592935e741a6c2c63cdd2b25..680c9f595014d6bbf867373f23eeb01e2fe92010 100644 (file)
@@ -54,7 +54,7 @@ TEST(Signals, SignalsWorkOnExit)
 
     bool signalFired;
     {
-        std::unique_lock<std::mutex> locker(static_cast<ReflectedThread&>(receiverThread.get()).m_mutex);
+        std::unique_lock<WordLock> locker(static_cast<ReflectedThread&>(receiverThread.get()).m_mutex);
         receiverShouldKeepRunning.store(false);
         EXPECT_FALSE(static_cast<ReflectedThread&>(receiverThread.get()).hasExited());
         sleep(1_s);