Modified: trunk/Source/WTF/ChangeLog (205858 => 205859)
--- trunk/Source/WTF/ChangeLog 2016-09-13 15:02:44 UTC (rev 205858)
+++ trunk/Source/WTF/ChangeLog 2016-09-13 15:59:25 UTC (rev 205859)
@@ -1,3 +1,21 @@
+2016-09-12 Filip Pizlo <[email protected]>
+
+ ParkingLot is going to have a bad time with threads dying
+ https://bugs.webkit.org/show_bug.cgi?id=161893
+
+ Reviewed by Michael Saboff.
+
+ If a thread dies right as it falls out of parkConditionally, then unparkOne() and friends
+ might die because they will dereference a deallocated ThreadData.
+
+ The solution is to ref-count ThreadData's. When unparkOne() and friends want to hold onto a
+ ThreadData past the queue lock, they can use RefPtr<>.
+
+ * wtf/ParkingLot.cpp:
+ (WTF::ParkingLot::unparkOne):
+ (WTF::ParkingLot::unparkOneImpl):
+ (WTF::ParkingLot::unparkAll):
+
2016-09-12 Chris Dumez <[email protected]>
Fix post-landing review comments after r205787
Modified: trunk/Source/WTF/wtf/ParkingLot.cpp (205858 => 205859)
--- trunk/Source/WTF/wtf/ParkingLot.cpp 2016-09-13 15:02:44 UTC (rev 205858)
+++ trunk/Source/WTF/wtf/ParkingLot.cpp 2016-09-13 15:59:25 UTC (rev 205859)
@@ -45,7 +45,7 @@
const bool verbose = false;
-struct ThreadData {
+struct ThreadData : public ThreadSafeRefCounted<ThreadData> {
WTF_MAKE_FAST_ALLOCATED;
public:
@@ -245,7 +245,6 @@
}
};
-ThreadSpecific<ThreadData>* threadData;
Atomic<Hashtable*> hashtable;
Atomic<unsigned> numThreads;
@@ -448,14 +447,20 @@
ThreadData* myThreadData()
{
+ static ThreadSpecific<RefPtr<ThreadData>>* threadData;
static std::once_flag initializeOnce;
std::call_once(
initializeOnce,
[] {
- threadData = new ThreadSpecific<ThreadData>();
+ threadData = new ThreadSpecific<RefPtr<ThreadData>>();
});
-
- return *threadData;
+
+ RefPtr<ThreadData>& result = **threadData;
+
+ if (!result)
+ result = adoptRef(new ThreadData());
+
+ return result.get();
}
template<typename Functor>
@@ -659,7 +664,7 @@
UnparkResult result;
- ThreadData* threadData = nullptr;
+ RefPtr<ThreadData> threadData;
result.mayHaveMoreThreads = dequeue(
address,
BucketMode::EnsureNonEmpty,
@@ -697,7 +702,7 @@
if (verbose)
dataLog(toString(currentThread(), ": unparking one the hard way.\n"));
- ThreadData* threadData = nullptr;
+ RefPtr<ThreadData> threadData;
bool timeToBeFair = false;
dequeue(
address,
@@ -738,7 +743,7 @@
if (verbose)
dataLog(toString(currentThread(), ": unparking all from ", RawPointer(address), ".\n"));
- Vector<ThreadData*, 8> threadDatas;
+ Vector<RefPtr<ThreadData>, 8> threadDatas;
dequeue(
address,
BucketMode::IgnoreEmpty,
@@ -752,9 +757,9 @@
},
[] (bool) { });
- for (ThreadData* threadData : threadDatas) {
+ for (RefPtr<ThreadData>& threadData : threadDatas) {
if (verbose)
- dataLog(toString(currentThread(), ": unparking ", RawPointer(threadData), " with address ", RawPointer(threadData->address), "\n"));
+ dataLog(toString(currentThread(), ": unparking ", RawPointer(threadData.get()), " with address ", RawPointer(threadData->address), "\n"));
ASSERT(threadData->address);
{
std::unique_lock<std::mutex> locker(threadData->parkingLock);
