[webkit-changes] [158378] tags/Safari-538.4/Source/JavaScriptCore

Thu, 31 Oct 2013 11:38:54 -0700

Title: [158378] tags/Safari-538.4/Source/_javascript_Core

Diff

Modified: tags/Safari-538.4/Source/_javascript_Core/ChangeLog (158377 => 158378)


--- tags/Safari-538.4/Source/_javascript_Core/ChangeLog	2013-10-31 18:33:47 UTC (rev 158377)
+++ tags/Safari-538.4/Source/_javascript_Core/ChangeLog	2013-10-31 18:40:14 UTC (rev 158378)
@@ -1,3 +1,48 @@
+2013-10-31  Lucas Forschler  <[email protected]>
+
+        Merge r158341
+
+    2013-10-30  Filip Pizlo  <[email protected]>
+
+            Assertion failure in js/dom/global-constructors-attributes-dedicated-worker.html
+            https://bugs.webkit.org/show_bug.cgi?id=123551
+            <rdar://problem/15356238>
+
+            Reviewed by Mark Hahnenberg.
+
+            WatchpointSets have always had this "fire everything on deletion" policy because it
+            seemed like a good fail-safe at the time I first implemented WatchpointSets. But
+            it's actually causing bugs rather than providing safety:
+
+            - Everyone who registers Watchpoints with WatchpointSets have separate mechanisms
+              for either keeping the WatchpointSets alive or noticing when they are collected.
+              So this wasn't actually providing any safety.
+
+              One example of this is Structures, where:
+
+              - CodeBlocks that register Watchpoints on Structure's WatchpointSet will also
+                register weak references to the Structure, and the GC will jettison a CodeBlock
+                if the Structure(s) it cares about dies.
+
+              - StructureStubInfos that register Watchpoints on Structure's WatchpointSet will
+                also be cleared by GC if the Structures die.
+
+            - The WatchpointSet destructor would get invoked from finalization/destruction.
+              This would then cause CodeBlock::jettison() to be called on a CodeBlock, but that
+              method requires doing things that access heap objects. This would usually cause
+              problems on VM destruction, since then the CodeBlocks would still be alive but the
+              whole heap would be destroyed.
+
+            This also ensures that CodeBlock::jettison() cannot cause a GC. This is safe since
+            that method doesn't really allocate objects, and it is likely necessary because
+            jettison() may be called from deep in the stack.
+
+            * bytecode/CodeBlock.cpp:
+            (JSC::CodeBlock::jettison):
+            * bytecode/Watchpoint.cpp:
+            (JSC::WatchpointSet::~WatchpointSet):
+            * bytecode/Watchpoint.h:
+
 2013-10-30  Mark Lam  <[email protected]>
 
         Unreviewed, fix C Loop LLINT build.

Modified: tags/Safari-538.4/Source/_javascript_Core/bytecode/CodeBlock.cpp (158377 => 158378)


--- tags/Safari-538.4/Source/_javascript_Core/bytecode/CodeBlock.cpp	2013-10-31 18:33:47 UTC (rev 158377)
+++ tags/Safari-538.4/Source/_javascript_Core/bytecode/CodeBlock.cpp	2013-10-31 18:40:14 UTC (rev 158378)
@@ -2817,7 +2817,7 @@
         dataLog(".\n");
     }
     
-    DeferGC deferGC(*m_heap);
+    DeferGCForAWhile deferGC(*m_heap);
     RELEASE_ASSERT(JITCode::isOptimizingJIT(jitType()));
     
     // We want to accomplish two things here:

Modified: tags/Safari-538.4/Source/_javascript_Core/bytecode/Watchpoint.cpp (158377 => 158378)


--- tags/Safari-538.4/Source/_javascript_Core/bytecode/Watchpoint.cpp	2013-10-31 18:33:47 UTC (rev 158377)
+++ tags/Safari-538.4/Source/_javascript_Core/bytecode/Watchpoint.cpp	2013-10-31 18:40:14 UTC (rev 158378)
@@ -46,10 +46,12 @@
 
 WatchpointSet::~WatchpointSet()
 {
-    // Fire all watchpoints. This is necessary because it is possible, say with
-    // structure watchpoints, for the watchpoint set owner to die while the
-    // watchpoint owners are still live.
-    fireAllWatchpoints();
+    // Remove all watchpoints, so that they don't try to remove themselves. Note that we
+    // don't fire watchpoints on deletion. We assume that any code that is interested in
+    // watchpoints already also separately has a mechanism to make sure that the code is
+    // either keeping the watchpoint set's owner alive, or does some weak reference thing.
+    while (!m_set.isEmpty())
+        m_set.begin()->remove();
 }
 
 void WatchpointSet::add(Watchpoint* watchpoint)

Modified: tags/Safari-538.4/Source/_javascript_Core/bytecode/Watchpoint.h (158377 => 158378)


--- tags/Safari-538.4/Source/_javascript_Core/bytecode/Watchpoint.h	2013-10-31 18:33:47 UTC (rev 158377)
+++ tags/Safari-538.4/Source/_javascript_Core/bytecode/Watchpoint.h	2013-10-31 18:40:14 UTC (rev 158378)
@@ -53,7 +53,7 @@
     friend class LLIntOffsetsExtractor;
 public:
     WatchpointSet(InitialWatchpointSetMode);
-    ~WatchpointSet();
+    ~WatchpointSet(); // Note that this will not fire any of the watchpoints; if you need to know when a WatchpointSet dies then you need a separate mechanism for this.
     
     // It is safe to call this from another thread.  It may return true
     // even if the set actually had been invalidated, but that ought to happen

_______________________________________________
webkit-changes mailing list
[email protected]
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to