WordLock doesn't need per-thread data
https://bugs.webkit.org/show_bug.cgi?id=185119
Reviewed by Yusuke Suzuki.
The stack is per-thread data, so we can stack-allocate our ThreadData.
This eliminates malloc() and high-level WTF threading primitives from
WordLock, making WordLock more portable to non-WTF code, including
bmalloc.
(NOTE: This patch makes the bug fixed in r231148 100% reproducible.)
* wtf/WordLock.cpp:
(WTF::WordLock::lockSlow): Allocate ThreadData on the stack.
git-svn-id: http://svn.webkit.org/repository/webkit/trunk@231151 268f45cc-cd09-0410-ab3c-d52691b4dbfc
diff --git a/Source/WTF/ChangeLog b/Source/WTF/ChangeLog
index 666e6df..3da61f6 100644
--- a/Source/WTF/ChangeLog
+++ b/Source/WTF/ChangeLog
@@ -1,3 +1,21 @@
+2018-04-29 Geoffrey Garen <ggaren@apple.com>
+
+ WordLock doesn't need per-thread data
+ https://bugs.webkit.org/show_bug.cgi?id=185119
+
+ Reviewed by Yusuke Suzuki.
+
+ The stack is per-thread data, so we can stack-allocate our ThreadData.
+
+ This eliminates malloc() and high-level WTF threading primitives from
+ WordLock, making WordLock more portable to non-WTF code, including
+ bmalloc.
+
+ (NOTE: This patch makes the bug fixed in r231148 100% reproducible.)
+
+ * wtf/WordLock.cpp:
+ (WTF::WordLock::lockSlow): Allocate ThreadData on the stack.
+
2018-04-28 Geoffrey Garen <ggaren@apple.com>
Fixed a very unlikely race condition in WTF::WordLock
diff --git a/Source/WTF/wtf/WordLock.cpp b/Source/WTF/wtf/WordLock.cpp
index cfe0905..febb3b6 100644
--- a/Source/WTF/wtf/WordLock.cpp
+++ b/Source/WTF/wtf/WordLock.cpp
@@ -26,9 +26,7 @@
#include "config.h"
#include "WordLock.h"
-#include "ThreadSpecific.h"
#include "Threading.h"
-#include "ThreadingPrimitives.h"
#include <condition_variable>
#include <mutex>
#include <thread>
@@ -60,20 +58,6 @@
ThreadData* queueTail { nullptr };
};
-ThreadSpecific<ThreadData, CanBeGCThread::True>* threadData;
-
-ThreadData* myThreadData()
-{
- static std::once_flag initializeOnce;
- std::call_once(
- initializeOnce,
- [] {
- threadData = new ThreadSpecific<ThreadData, CanBeGCThread::True>();
- });
-
- return *threadData;
-}
-
} // anonymous namespace
NEVER_INLINE void WordLock::lockSlow()
@@ -106,12 +90,8 @@
// Need to put ourselves on the queue. Create the queue if one does not exist. This requries
// owning the queue for a little bit. The lock that controls the queue is itself a spinlock.
- // But before we acquire the queue spinlock, we make sure that we have a ThreadData for this
- // thread.
- ThreadData* me = myThreadData();
- ASSERT(!me->shouldPark);
- ASSERT(!me->nextInQueue);
- ASSERT(!me->queueTail);
+
+ ThreadData me;
// Reload the current word value, since some time may have passed.
currentWordValue = m_word.load();
@@ -125,15 +105,15 @@
continue;
}
- me->shouldPark = true;
+ me.shouldPark = true;
// We own the queue. Nobody can enqueue or dequeue until we're done. Also, it's not possible
// to release the WordLock while we hold the queue lock.
ThreadData* queueHead = bitwise_cast<ThreadData*>(currentWordValue & ~queueHeadMask);
if (queueHead) {
// Put this thread at the end of the queue.
- queueHead->queueTail->nextInQueue = me;
- queueHead->queueTail = me;
+ queueHead->queueTail->nextInQueue = &me;
+ queueHead->queueTail = &me;
// Release the queue lock.
currentWordValue = m_word.load();
@@ -143,8 +123,8 @@
m_word.store(currentWordValue & ~isQueueLockedBit);
} else {
// Make this thread be the queue-head.
- queueHead = me;
- me->queueTail = me;
+ queueHead = &me;
+ me.queueTail = &me;
// Release the queue lock and install ourselves as the head. No need for a CAS loop, since
// we own the queue lock.
@@ -164,14 +144,14 @@
// releasing thread holds me's parkingLock.
{
- std::unique_lock<std::mutex> locker(me->parkingLock);
- while (me->shouldPark)
- me->parkingCondition.wait(locker);
+ std::unique_lock<std::mutex> locker(me.parkingLock);
+ while (me.shouldPark)
+ me.parkingCondition.wait(locker);
}
- ASSERT(!me->shouldPark);
- ASSERT(!me->nextInQueue);
- ASSERT(!me->queueTail);
+ ASSERT(!me.shouldPark);
+ ASSERT(!me.nextInQueue);
+ ASSERT(!me.queueTail);
// Now we can loop around and try to acquire the lock again.
}
