Changeset 181486 in webkit

Timestamp:

Mar 13, 2015, 1:14:39 PM (11 years ago)

Author:

ggaren@apple.com

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.

Location:

trunk/Source/JavaScriptCore

Files:

• ChangeLog (modified) (1 diff)
• heap/Heap.cpp (modified) (2 diffs)
• heap/IncrementalSweeper.cpp (modified) (1 diff)

trunk/Source/JavaScriptCore/ChangeLog

@@ +1 @@
+2015-03-12  Geoffrey Garen  <ggaren@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.lam@apple.com>
 

trunk/Source/JavaScriptCore/heap/Heap.cpp

@@ -989 +989 @@
 
     SamplingRegion samplingRegion("Garbage Collection: Sweeping");
+
+    DeferGCForAWhile deferGC(*this);
     m_objectSpace.sweep();
     m_objectSpace.shrink();
@@ -1294 +1296 @@
     if (Options::recordGCPauseTimes())
         HeapStatistics::recordGCPauseTime(gcStartTime, gcEndTime);
+
+    if (Options::useZombieMode())
+        zombifyDeadObjects();
+
+    if (Options::objectsAreImmortal())
+        markDeadObjects();
+
+    if (Options::showObjectStatistics())
+        HeapStatistics::showObjectStatistics(this);
+
+    if (Options::logGC() == GCLogging::Verbose)
+        GCLogging::dumpObjectGraph(this);
+
     RELEASE_ASSERT(m_operationInProgress == EdenCollection || m_operationInProgress == FullCollection);
-
     m_operationInProgress = NoOperation;
     JAVASCRIPTCORE_GC_END();
-
-    if (Options::useZombieMode())
-        zombifyDeadObjects();
-
-    if (Options::objectsAreImmortal())
-        markDeadObjects();
-
-    if (Options::showObjectStatistics())
-        HeapStatistics::showObjectStatistics(this);
-
-    if (Options::logGC() == GCLogging::Verbose)
-        GCLogging::dumpObjectGraph(this);
 }
 

trunk/Source/JavaScriptCore/heap/IncrementalSweeper.cpp

@@ -96 +96 @@
             continue;
 
+        DeferGCForAWhile deferGC(m_vm->heap);
         block->sweep();
         m_vm->heap.objectSpace().freeOrShrinkBlock(block);