[webkit-changes] [181486] trunk/Source/JavaScriptCore

[ggaren]

Title: [181486] trunk/Source/_javascript_Core

Revision

181486

Author

gga...@apple.com

Date

2015-03-13 13:14:39 -0700 (Fri, 13 Mar 2015)

Log Message

Prohibit GC while sweeping
https://bugs.webkit.org/show_bug.cgi?id=142638

Reviewed by Andreas Kling.

I noticed in https://bugs.webkit.org/show_bug.cgi?id=142636 that a GC
could trigger a sweep which could trigger another GC. Yo Dawg.

I tried to figure out whether this could cause problems or not and it
made me cross-eyed.

(Some clients like to report extra memory cost during deallocation as a
way to indicate that the GC now owns something exclusively. It's
arguably a bug to communicate with the GC in this way, but we shouldn't
do crazy when this happens.)

This patch makes explicit the fact that we don't allow GC while sweeping.

Usually, sweeping implicitly defers GC by virtue of happening during
allocation. But not always.

* heap/Heap.cpp:
(JSC::Heap::collectAllGarbage): Defer GC while sweeping due to an
explicit GC request.

(JSC::Heap::didFinishCollection): Make sure that zombifying sweep
defers GC by not returning to the non-GC state until we're all done.

* heap/IncrementalSweeper.cpp:
(JSC::IncrementalSweeper::sweepNextBlock): Defer GC while sweeping due
to a timer.

Modified Paths

• trunk/Source/_javascript_Core/ChangeLog
• trunk/Source/_javascript_Core/heap/Heap.cpp
• trunk/Source/_javascript_Core/heap/IncrementalSweeper.cpp

Diff

Modified: trunk/Source/_javascript_Core/ChangeLog (181485 => 181486)

--- trunk/Source/_javascript_Core/ChangeLog	2015-03-13 20:04:29 UTC (rev 181485)
+++ trunk/Source/_javascript_Core/ChangeLog	2015-03-13 20:14:39 UTC (rev 181486)
@@ -1,3 +1,37 @@
+2015-03-12  Geoffrey Garen  <gga...@apple.com>
+
+        Prohibit GC while sweeping
+        https://bugs.webkit.org/show_bug.cgi?id=142638
+
+        Reviewed by Andreas Kling.
+
+        I noticed in https://bugs.webkit.org/show_bug.cgi?id=142636 that a GC
+        could trigger a sweep which could trigger another GC. Yo Dawg.
+
+        I tried to figure out whether this could cause problems or not and it
+        made me cross-eyed.
+
+        (Some clients like to report extra memory cost during deallocation as a
+        way to indicate that the GC now owns something exclusively. It's
+        arguably a bug to communicate with the GC in this way, but we shouldn't
+        do crazy when this happens.)
+
+        This patch makes explicit the fact that we don't allow GC while sweeping.
+
+        Usually, sweeping implicitly defers GC by virtue of happening during
+        allocation. But not always.
+
+        * heap/Heap.cpp:
+        (JSC::Heap::collectAllGarbage): Defer GC while sweeping due to an
+        explicit GC request.
+
+        (JSC::Heap::didFinishCollection): Make sure that zombifying sweep
+        defers GC by not returning to the non-GC state until we're all done.
+
+        * heap/IncrementalSweeper.cpp:
+        (JSC::IncrementalSweeper::sweepNextBlock): Defer GC while sweeping due
+        to a timer.
+
 2015-03-13  Mark Lam  <mark....@apple.com>
 
         Replace TCSpinLock with a new WTF::SpinLock based on WTF::Atomic.

Modified: trunk/Source/_javascript_Core/heap/Heap.cpp (181485 => 181486)

--- trunk/Source/_javascript_Core/heap/Heap.cpp	2015-03-13 20:04:29 UTC (rev 181485)
+++ trunk/Source/_javascript_Core/heap/Heap.cpp	2015-03-13 20:14:39 UTC (rev 181486)
@@ -988,6 +988,8 @@
     collect(FullCollection);
 
     SamplingRegion samplingRegion("Garbage Collection: Sweeping");
+
+    DeferGCForAWhile deferGC(*this);
     m_objectSpace.sweep();
     m_objectSpace.shrink();
 }
@@ -1293,11 +1295,7 @@
 
     if (Options::recordGCPauseTimes())
         HeapStatistics::recordGCPauseTime(gcStartTime, gcEndTime);
-    RELEASE_ASSERT(m_operationInProgress == EdenCollection || m_operationInProgress == FullCollection);
 
-    m_operationInProgress = NoOperation;
-    _javascript_CORE_GC_END();
-
     if (Options::useZombieMode())
         zombifyDeadObjects();
 
@@ -1309,6 +1307,10 @@
 
     if (Options::logGC() == GCLogging::Verbose)
         GCLogging::dumpObjectGraph(this);
+
+    RELEASE_ASSERT(m_operationInProgress == EdenCollection || m_operationInProgress == FullCollection);
+    m_operationInProgress = NoOperation;
+    _javascript_CORE_GC_END();
 }
 
 void Heap::resumeCompilerThreads()

Modified: trunk/Source/_javascript_Core/heap/IncrementalSweeper.cpp (181485 => 181486)

--- trunk/Source/_javascript_Core/heap/IncrementalSweeper.cpp	2015-03-13 20:04:29 UTC (rev 181485)
+++ trunk/Source/_javascript_Core/heap/IncrementalSweeper.cpp	2015-03-13 20:14:39 UTC (rev 181486)
@@ -95,6 +95,7 @@
         if (!block->needsSweeping())
             continue;
 
+        DeferGCForAWhile deferGC(m_vm->heap);
         block->sweep();
         m_vm->heap.objectSpace().freeOrShrinkBlock(block);
         return;

_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes