Changeset 207071 in webkit

Timestamp:

Oct 11, 2016 2:23:28 AM (8 years ago)

Author:

Carlos Garcia Campos

Message:

Merge r205859 - 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):

Location:

releases/WebKitGTK/webkit-2.14/Source/WTF

Files:

• ChangeLog (modified) (1 diff)
• wtf/ParkingLot.cpp (modified) (7 diffs)

releases/WebKitGTK/webkit-2.14/Source/WTF/ChangeLog

@@ +1 @@
+2016-09-12  Filip Pizlo  <fpizlo@apple.com>
+
+        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  Yusuke Suzuki  <utatane.tea@gmail.com>
 

releases/WebKitGTK/webkit-2.14/Source/WTF/wtf/ParkingLot.cpp

@@ -46 +46 @@
 const bool verbose = false;
 
-struct ThreadData {
+struct ThreadData : public ThreadSafeRefCounted<ThreadData> {
     WTF_MAKE_FAST_ALLOCATED;
 public:
@@ -246 +246 @@
 };
 
-ThreadSpecific<ThreadData>* threadData;
 Atomic<Hashtable*> hashtable;
 Atomic<unsigned> numThreads;
@@ -449 +448 @@
 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();
 }
 
@@ -660 +665 @@
     UnparkResult result;
 
-    ThreadData* threadData = nullptr;
+    RefPtr<ThreadData> threadData;
     result.mayHaveMoreThreads = dequeue(
         address,
@@ -698 +703 @@
         dataLog(toString(currentThread(), ": unparking one the hard way.\n"));
     
-    ThreadData* threadData = nullptr;
+    RefPtr<ThreadData> threadData;
     bool timeToBeFair = false;
     dequeue(
@@ -739 +744 @@
         dataLog(toString(currentThread(), ": unparking all from ", RawPointer(address), ".\n"));
     
-    Vector<ThreadData*, 8> threadDatas;
+    Vector<RefPtr<ThreadData>, 8> threadDatas;
     dequeue(
         address,
@@ -753 +758 @@
         [] (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);
         {