Title: [248239] releases/WebKitGTK/webkit-2.24/Source/_javascript_Core
- Revision
- 248239
- Author
- [email protected]
- Date
- 2019-08-03 20:23:29 -0700 (Sat, 03 Aug 2019)
Log Message
Merge r247426 - Concurrent GC should not rely on current phase to determine if it's safe to steal conn
https://bugs.webkit.org/show_bug.cgi?id=199786
<rdar://problem/52505197>
Reviewed by Saam Barati.
In r246507, we fixed a race condition in the concurrent GC where the mutator might steal
the conn from the collector thread while it transitions from the End phase to NotRunning.
However, that fix was not sufficient. In the case that the mutator steals the conn, and the
execution interleaves long enough for the mutator to progress to a different collection phase,
the collector will resume in a phase other than NotRunning, and hence the check added to
NotRunning will not suffice. To fix that, we add a new variable to track whether the collector
thread is running (m_collectorThreadIsRunning) and use it to determine whether it's safe to
steal the conn, rather than relying on m_currentPhase.
* heap/Heap.cpp:
(JSC::Heap::runNotRunningPhase):
(JSC::Heap::requestCollection):
* heap/Heap.h:
Modified Paths
Diff
Modified: releases/WebKitGTK/webkit-2.24/Source/_javascript_Core/ChangeLog (248238 => 248239)
--- releases/WebKitGTK/webkit-2.24/Source/_javascript_Core/ChangeLog 2019-08-04 03:23:27 UTC (rev 248238)
+++ releases/WebKitGTK/webkit-2.24/Source/_javascript_Core/ChangeLog 2019-08-04 03:23:29 UTC (rev 248239)
@@ -1,3 +1,25 @@
+2019-07-15 Tadeu Zagallo <[email protected]>
+
+ Concurrent GC should not rely on current phase to determine if it's safe to steal conn
+ https://bugs.webkit.org/show_bug.cgi?id=199786
+ <rdar://problem/52505197>
+
+ Reviewed by Saam Barati.
+
+ In r246507, we fixed a race condition in the concurrent GC where the mutator might steal
+ the conn from the collector thread while it transitions from the End phase to NotRunning.
+ However, that fix was not sufficient. In the case that the mutator steals the conn, and the
+ execution interleaves long enough for the mutator to progress to a different collection phase,
+ the collector will resume in a phase other than NotRunning, and hence the check added to
+ NotRunning will not suffice. To fix that, we add a new variable to track whether the collector
+ thread is running (m_collectorThreadIsRunning) and use it to determine whether it's safe to
+ steal the conn, rather than relying on m_currentPhase.
+
+ * heap/Heap.cpp:
+ (JSC::Heap::runNotRunningPhase):
+ (JSC::Heap::requestCollection):
+ * heap/Heap.h:
+
2019-06-17 Tadeu Zagallo <[email protected]>
Concurrent GC should check the conn before starting a new collection cycle
Modified: releases/WebKitGTK/webkit-2.24/Source/_javascript_Core/heap/Heap.cpp (248238 => 248239)
--- releases/WebKitGTK/webkit-2.24/Source/_javascript_Core/heap/Heap.cpp 2019-08-04 03:23:27 UTC (rev 248238)
+++ releases/WebKitGTK/webkit-2.24/Source/_javascript_Core/heap/Heap.cpp 2019-08-04 03:23:29 UTC (rev 248239)
@@ -250,8 +250,11 @@
m_heap.notifyThreadStopping(locker);
return PollResult::Stop;
}
- if (m_heap.shouldCollectInCollectorThread(locker))
+ if (m_heap.shouldCollectInCollectorThread(locker)) {
+ m_heap.m_collectorThreadIsRunning = true;
return PollResult::Work;
+ }
+ m_heap.m_collectorThreadIsRunning = false;
return PollResult::Wait;
}
@@ -266,6 +269,11 @@
WTF::registerGCThread(GCThreadType::Main);
}
+ void threadIsStopping(const AbstractLocker&) override
+ {
+ m_heap.m_collectorThreadIsRunning = false;
+ }
+
private:
Heap& m_heap;
};
@@ -1193,9 +1201,6 @@
auto locker = holdLock(*m_threadLock);
if (m_requests.isEmpty())
return false;
- // Check if the mutator has stolen the conn while the collector transitioned from End to NotRunning
- if (conn == GCConductor::Collector && !!(m_worldState.load() & mutatorHasConnBit))
- return false;
}
return changePhase(conn, CollectorPhase::Begin);
@@ -2088,7 +2093,7 @@
// right now. This is an optimization that prevents the collector thread from ever starting in most
// cases.
ASSERT(m_lastServedTicket <= m_lastGrantedTicket);
- if ((m_lastServedTicket == m_lastGrantedTicket) && (m_currentPhase == CollectorPhase::NotRunning)) {
+ if ((m_lastServedTicket == m_lastGrantedTicket) && !m_collectorThreadIsRunning) {
if (false)
dataLog("Taking the conn.\n");
m_worldState.exchangeOr(mutatorHasConnBit);
Modified: releases/WebKitGTK/webkit-2.24/Source/_javascript_Core/heap/Heap.h (248238 => 248239)
--- releases/WebKitGTK/webkit-2.24/Source/_javascript_Core/heap/Heap.h 2019-08-04 03:23:27 UTC (rev 248238)
+++ releases/WebKitGTK/webkit-2.24/Source/_javascript_Core/heap/Heap.h 2019-08-04 03:23:29 UTC (rev 248239)
@@ -713,6 +713,7 @@
CollectorPhase m_lastPhase { CollectorPhase::NotRunning };
CollectorPhase m_currentPhase { CollectorPhase::NotRunning };
CollectorPhase m_nextPhase { CollectorPhase::NotRunning };
+ bool m_collectorThreadIsRunning { false };
bool m_threadShouldStop { false };
bool m_threadIsStopping { false };
bool m_mutatorDidRun { true };
_______________________________________________
webkit-changes mailing list
[email protected]
https://lists.webkit.org/mailman/listinfo/webkit-changes
