Title: [167692] trunk/Source/_javascript_Core
Revision
167692
Author
mark....@apple.com
Date
2014-04-22 17:36:49 -0700 (Tue, 22 Apr 2014)

Log Message

DFG::Worklist should acquire the m_lock before iterating DFG plans.
<https://webkit.org/b/132032>

Reviewed by Filip Pizlo.

Currently, there's a rightToRun mechanism that ensures that no compilation
threads are running when the GC is iterating through the DFG worklists.
However, this does not prevent a Worker thread from doing a DFG compilation
and modifying the plans in the worklists thereby invalidating the plan
iterator that the GC is using.  This patch fixes the issue by acquiring
the worklist m_lock before iterating the worklist plans.

This issue was uncovered by running the fast/workers layout tests with
COLLECT_ON_EVERY_ALLOCATION enabled.

* dfg/DFGWorklist.cpp:
(JSC::DFG::Worklist::isActiveForVM):
(JSC::DFG::Worklist::visitChildren):

Modified Paths

Diff

Modified: trunk/Source/_javascript_Core/ChangeLog (167691 => 167692)


--- trunk/Source/_javascript_Core/ChangeLog	2014-04-22 23:54:49 UTC (rev 167691)
+++ trunk/Source/_javascript_Core/ChangeLog	2014-04-23 00:36:49 UTC (rev 167692)
@@ -1,3 +1,24 @@
+2014-04-22  Mark Lam  <mark....@apple.com>
+
+        DFG::Worklist should acquire the m_lock before iterating DFG plans.
+        <https://webkit.org/b/132032>
+
+        Reviewed by Filip Pizlo.
+
+        Currently, there's a rightToRun mechanism that ensures that no compilation
+        threads are running when the GC is iterating through the DFG worklists.
+        However, this does not prevent a Worker thread from doing a DFG compilation
+        and modifying the plans in the worklists thereby invalidating the plan
+        iterator that the GC is using.  This patch fixes the issue by acquiring
+        the worklist m_lock before iterating the worklist plans.
+
+        This issue was uncovered by running the fast/workers layout tests with
+        COLLECT_ON_EVERY_ALLOCATION enabled.
+
+        * dfg/DFGWorklist.cpp:
+        (JSC::DFG::Worklist::isActiveForVM):
+        (JSC::DFG::Worklist::visitChildren):
+
 2014-04-22  Brent Fulgham  <bfulg...@apple.com>
 
         [Win] Support Python 2.7 in Cygwin

Modified: trunk/Source/_javascript_Core/dfg/DFGWorklist.cpp (167691 => 167692)


--- trunk/Source/_javascript_Core/dfg/DFGWorklist.cpp	2014-04-22 23:54:49 UTC (rev 167691)
+++ trunk/Source/_javascript_Core/dfg/DFGWorklist.cpp	2014-04-23 00:36:49 UTC (rev 167692)
@@ -76,6 +76,7 @@
 
 bool Worklist::isActiveForVM(VM& vm) const
 {
+    MutexLocker locker(m_lock);
     PlanMap::const_iterator end = m_plans.end();
     for (PlanMap::const_iterator iter = m_plans.begin(); iter != end; ++iter) {
         if (&iter->value->vm == &vm)
@@ -222,14 +223,21 @@
 void Worklist::visitChildren(SlotVisitor& visitor, CodeBlockSet& codeBlocks)
 {
     VM* vm = visitor.heap()->vm();
-    for (PlanMap::iterator iter = m_plans.begin(); iter != m_plans.end(); ++iter) {
-        Plan* plan = iter->value.get();
-        if (&plan->vm != vm)
-            continue;
-        iter->key.visitChildren(codeBlocks);
-        iter->value->visitChildren(visitor, codeBlocks);
+    {
+        MutexLocker locker(m_lock);
+        for (PlanMap::iterator iter = m_plans.begin(); iter != m_plans.end(); ++iter) {
+            Plan* plan = iter->value.get();
+            if (&plan->vm != vm)
+                continue;
+            iter->key.visitChildren(codeBlocks);
+            iter->value->visitChildren(visitor, codeBlocks);
+        }
     }
     
+    // This loop doesn't need further locking because:
+    // (1) no new threads can be added to m_threads. Hence, it is immutable and needs no locks.
+    // (2) ThreadData::m_safepoint is protected by that thread's m_rightToRun which we must be
+    //     holding here because of a prior call to suspendAllThreads().
     for (unsigned i = m_threads.size(); i--;) {
         ThreadData* data = ""
         Safepoint* safepoint = data->m_safepoint;
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to