Title: [231151] trunk/Source/WTF
- Revision
- 231151
- Author
- [email protected]
- Date
- 2018-04-29 10:29:13 -0700 (Sun, 29 Apr 2018)
Log Message
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.
Modified Paths
Diff
Modified: trunk/Source/WTF/ChangeLog (231150 => 231151)
--- trunk/Source/WTF/ChangeLog 2018-04-29 16:09:38 UTC (rev 231150)
+++ trunk/Source/WTF/ChangeLog 2018-04-29 17:29:13 UTC (rev 231151)
@@ -1,3 +1,21 @@
+2018-04-29 Geoffrey Garen <[email protected]>
+
+ 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 <[email protected]>
Fixed a very unlikely race condition in WTF::WordLock
Modified: trunk/Source/WTF/wtf/WordLock.cpp (231150 => 231151)
--- trunk/Source/WTF/wtf/WordLock.cpp 2018-04-29 16:09:38 UTC (rev 231150)
+++ trunk/Source/WTF/wtf/WordLock.cpp 2018-04-29 17:29:13 UTC (rev 231151)
@@ -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,13 +90,9 @@
// 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,7 +105,7 @@
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.
@@ -132,8 +112,8 @@
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.
}
_______________________________________________
webkit-changes mailing list
[email protected]
https://lists.webkit.org/mailman/listinfo/webkit-changes
