Title: [246507] trunk/Source/_javascript_Core
- Revision
- 246507
- Author
- tzaga...@apple.com
- Date
- 2019-06-17 11:54:30 -0700 (Mon, 17 Jun 2019)
Log Message
Concurrent GC should check the conn before starting a new collection cycle
https://bugs.webkit.org/show_bug.cgi?id=198913
<rdar://problem/49515149>
Reviewed by Filip Pizlo.
Heap::requestCollection tries to steal the conn as an optimization to avoid waking up the collector
thread if it's idle. We determine if the collector is idle by ensuring that there are no pending collections
and that the current GC phase is NotRunning. However, that's not safe immediately after the concurrent
GC has finished processing the last pending request. The collector thread will runEndPhase and immediately
start runNotRunningPhase, without checking if it still has the conn. If the mutator has stolen the conn in
the mean time, this will lead to both threads collecting concurrently, and eventually we'll crash in checkConn,
since the collector is running but doesn't have the conn anymore.
To solve this, we check if we still have the conn after holding the lock in runNotRunningPhase, in case the mutator
has stolen the conn. Ideally, we wouldn't let the mutator steal the conn in the first place, but that doesn't seem
trivial to determine.
* heap/Heap.cpp:
(JSC::Heap::runNotRunningPhase):
Modified Paths
Diff
Modified: trunk/Source/_javascript_Core/ChangeLog (246506 => 246507)
--- trunk/Source/_javascript_Core/ChangeLog 2019-06-17 18:53:31 UTC (rev 246506)
+++ trunk/Source/_javascript_Core/ChangeLog 2019-06-17 18:54:30 UTC (rev 246507)
@@ -1,3 +1,26 @@
+2019-06-17 Tadeu Zagallo <tzaga...@apple.com>
+
+ Concurrent GC should check the conn before starting a new collection cycle
+ https://bugs.webkit.org/show_bug.cgi?id=198913
+ <rdar://problem/49515149>
+
+ Reviewed by Filip Pizlo.
+
+ Heap::requestCollection tries to steal the conn as an optimization to avoid waking up the collector
+ thread if it's idle. We determine if the collector is idle by ensuring that there are no pending collections
+ and that the current GC phase is NotRunning. However, that's not safe immediately after the concurrent
+ GC has finished processing the last pending request. The collector thread will runEndPhase and immediately
+ start runNotRunningPhase, without checking if it still has the conn. If the mutator has stolen the conn in
+ the mean time, this will lead to both threads collecting concurrently, and eventually we'll crash in checkConn,
+ since the collector is running but doesn't have the conn anymore.
+
+ To solve this, we check if we still have the conn after holding the lock in runNotRunningPhase, in case the mutator
+ has stolen the conn. Ideally, we wouldn't let the mutator steal the conn in the first place, but that doesn't seem
+ trivial to determine.
+
+ * heap/Heap.cpp:
+ (JSC::Heap::runNotRunningPhase):
+
2019-06-17 Yusuke Suzuki <ysuz...@apple.com>
[JSC] Introduce DisposableCallSiteIndex to enforce type-safety
Modified: trunk/Source/_javascript_Core/heap/Heap.cpp (246506 => 246507)
--- trunk/Source/_javascript_Core/heap/Heap.cpp 2019-06-17 18:53:31 UTC (rev 246506)
+++ trunk/Source/_javascript_Core/heap/Heap.cpp 2019-06-17 18:54:30 UTC (rev 246507)
@@ -1234,6 +1234,9 @@
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);
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes
