<!DOCTYPE html PUBLIC "-//W3C//DTD XHTML 1.1//EN"
"http://www.w3.org/TR/xhtml11/DTD/xhtml11.dtd">
<html xmlns="http://www.w3.org/1999/xhtml">
<head><meta http-equiv="content-type" content="text/html; charset=utf-8" />
<title>[246746] branches/safari-607-branch/Source/JavaScriptCore</title>
</head>
<body>

<style type="text/css"><!--
#msg dl.meta { border: 1px #006 solid; background: #369; padding: 6px; color: #fff; }
#msg dl.meta dt { float: left; width: 6em; font-weight: bold; }
#msg dt:after { content:':';}
#msg dl, #msg dt, #msg ul, #msg li, #header, #footer, #logmsg { font-family: verdana,arial,helvetica,sans-serif; font-size: 10pt;  }
#msg dl a { font-weight: bold}
#msg dl a:link    { color:#fc3; }
#msg dl a:active  { color:#ff0; }
#msg dl a:visited { color:#cc6; }
h3 { font-family: verdana,arial,helvetica,sans-serif; font-size: 10pt; font-weight: bold; }
#msg pre { overflow: auto; background: #ffc; border: 1px #fa0 solid; padding: 6px; }
#logmsg { background: #ffc; border: 1px #fa0 solid; padding: 1em 1em 0 1em; }
#logmsg p, #logmsg pre, #logmsg blockquote { margin: 0 0 1em 0; }
#logmsg p, #logmsg li, #logmsg dt, #logmsg dd { line-height: 14pt; }
#logmsg h1, #logmsg h2, #logmsg h3, #logmsg h4, #logmsg h5, #logmsg h6 { margin: .5em 0; }
#logmsg h1:first-child, #logmsg h2:first-child, #logmsg h3:first-child, #logmsg h4:first-child, #logmsg h5:first-child, #logmsg h6:first-child { margin-top: 0; }
#logmsg ul, #logmsg ol { padding: 0; list-style-position: inside; margin: 0 0 0 1em; }
#logmsg ul { text-indent: -1em; padding-left: 1em; }#logmsg ol { text-indent: -1.5em; padding-left: 1.5em; }
#logmsg > ul, #logmsg > ol { margin: 0 0 1em 0; }
#logmsg pre { background: #eee; padding: 1em; }
#logmsg blockquote { border: 1px solid #fa0; border-left-width: 10px; padding: 1em 1em 0 1em; background: white;}
#logmsg dl { margin: 0; }
#logmsg dt { font-weight: bold; }
#logmsg dd { margin: 0; padding: 0 0 0.5em 0; }
#logmsg dd:before { content:'\00bb';}
#logmsg table { border-spacing: 0px; border-collapse: collapse; border-top: 4px solid #fa0; border-bottom: 1px solid #fa0; background: #fff; }
#logmsg table th { text-align: left; font-weight: normal; padding: 0.2em 0.5em; border-top: 1px dotted #fa0; }
#logmsg table td { text-align: right; border-top: 1px dotted #fa0; padding: 0.2em 0.5em; }
#logmsg table thead th { text-align: center; border-bottom: 1px solid #fa0; }
#logmsg table th.Corner { text-align: left; }
#logmsg hr { border: none 0; border-top: 2px dashed #fa0; height: 1px; }
#header, #footer { color: #fff; background: #636; border: 1px #300 solid; padding: 6px; }
#patch { width: 100%; }
#patch h4 {font-family: verdana,arial,helvetica,sans-serif;font-size:10pt;padding:8px;background:#369;color:#fff;margin:0;}
#patch .propset h4, #patch .binary h4 {margin:0;}
#patch pre {padding:0;line-height:1.2em;margin:0;}
#patch .diff {width:100%;background:#eee;padding: 0 0 10px 0;overflow:auto;}
#patch .propset .diff, #patch .binary .diff  {padding:10px 0;}
#patch span {display:block;padding:0 10px;}
#patch .modfile, #patch .addfile, #patch .delfile, #patch .propset, #patch .binary, #patch .copfile {border:1px solid #ccc;margin:10px 0;}
#patch ins {background:#dfd;text-decoration:none;display:block;padding:0 10px;}
#patch del {background:#fdd;text-decoration:none;display:block;padding:0 10px;}
#patch .lines, .info {color:#888;background:#fff;}
--></style>
<div id="msg">
<dl class="meta">
<dt>Revision</dt> <dd><a href="http://trac.webkit.org/projects/webkit/changeset/246746">246746</a></dd>
<dt>Author</dt> <dd>kocsen_chung@apple.com</dd>
<dt>Date</dt> <dd>2019-06-24 11:52:43 -0700 (Mon, 24 Jun 2019)</dd>
</dl>

<h3>Log Message</h3>
<pre>Cherry-pick <a href="http://trac.webkit.org/projects/webkit/changeset/246507">r246507</a>. rdar://problem/51927645

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

    git-svn-id: https://svn.webkit.org/repository/webkit/trunk@246507 268f45cc-cd09-0410-ab3c-d52691b4dbfc</pre>

<h3>Modified Paths</h3>
<ul>
<li><a href="#branchessafari607branchSourceJavaScriptCoreChangeLog">branches/safari-607-branch/Source/JavaScriptCore/ChangeLog</a></li>
<li><a href="#branchessafari607branchSourceJavaScriptCoreheapHeapcpp">branches/safari-607-branch/Source/JavaScriptCore/heap/Heap.cpp</a></li>
</ul>

</div>
<div id="patch">
<h3>Diff</h3>
<a id="branchessafari607branchSourceJavaScriptCoreChangeLog"></a>
<div class="modfile"><h4>Modified: branches/safari-607-branch/Source/JavaScriptCore/ChangeLog (246745 => 246746)</h4>
<pre class="diff"><span>
<span class="info">--- branches/safari-607-branch/Source/JavaScriptCore/ChangeLog       2019-06-24 18:52:38 UTC (rev 246745)
+++ branches/safari-607-branch/Source/JavaScriptCore/ChangeLog  2019-06-24 18:52:43 UTC (rev 246746)
</span><span class="lines">@@ -1,5 +1,56 @@
</span><span class="cx"> 2019-06-24  Kocsen Chung  <kocsen_chung@apple.com>
</span><span class="cx"> 
</span><ins>+        Cherry-pick r246507. rdar://problem/51927645
+
+    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):
+    
+    
+    git-svn-id: https://svn.webkit.org/repository/webkit/trunk@246507 268f45cc-cd09-0410-ab3c-d52691b4dbfc
+
+    2019-06-17  Tadeu Zagallo  <tzagallo@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-24  Kocsen Chung  <kocsen_chung@apple.com>
+
</ins><span class="cx">         Cherry-pick r246505. rdar://problem/51927642
</span><span class="cx"> 
</span><span class="cx">     [JSC] Introduce DisposableCallSiteIndex to enforce type-safety
</span></span></pre></div>
<a id="branchessafari607branchSourceJavaScriptCoreheapHeapcpp"></a>
<div class="modfile"><h4>Modified: branches/safari-607-branch/Source/JavaScriptCore/heap/Heap.cpp (246745 => 246746)</h4>
<pre class="diff"><span>
<span class="info">--- branches/safari-607-branch/Source/JavaScriptCore/heap/Heap.cpp   2019-06-24 18:52:38 UTC (rev 246745)
+++ branches/safari-607-branch/Source/JavaScriptCore/heap/Heap.cpp      2019-06-24 18:52:43 UTC (rev 246746)
</span><span class="lines">@@ -1196,6 +1196,9 @@
</span><span class="cx">         auto locker = holdLock(*m_threadLock);
</span><span class="cx">         if (m_requests.isEmpty())
</span><span class="cx">             return false;
</span><ins>+        // 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;
</ins><span class="cx">     }
</span><span class="cx">     
</span><span class="cx">     return changePhase(conn, CollectorPhase::Begin);
</span></span></pre>
</div>
</div>

</body>
</html>