Bug 937751, part 3 - Protect against reentrancy with when incrementally CCing. r=smaug
authorAndrew McCreight <continuation@gmail.com>
Tue, 03 Dec 2013 10:47:47 -0800
changeset 174288 aa6afd008433e97833bf80811678b4c5178e945e
parent 174287 dc1be6cd4cf650a8b22993c873177268f47d2cd0
child 174289 c6338d6ddd1fb6fef6be6de1a7c63e4864e05ae9
push id445
push userffxbld
push dateMon, 10 Mar 2014 22:05:19 +0000
treeherdermozilla-release@dc38b741b04e [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerssmaug
bugs937751
milestone28.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 937751, part 3 - Protect against reentrancy with when incrementally CCing. r=smaug Cycle collection protects against reentrancy by setting a flag to indicate a collection is in progress. With synchronous CC, it is okay to set this in BeginCollection, and clear it in CleanupAfterCollection. With ICC, this must be set and cleared in every slice, so I moved the fixing of it to Collect. I also changed the name of the variable, because we can be in the middle of an ICC without the CC being actively running, and it is only the latter we are worried about here.
xpcom/base/nsCycleCollector.cpp
--- a/xpcom/base/nsCycleCollector.cpp
+++ b/xpcom/base/nsCycleCollector.cpp
@@ -1001,17 +1001,17 @@ enum ccType {
 ////////////////////////////////////////////////////////////////////////
 // Top level structure for the cycle collector.
 ////////////////////////////////////////////////////////////////////////
 
 class nsCycleCollector : public MemoryMultiReporter
 {
     NS_DECL_ISUPPORTS
 
-    bool mCollectionInProgress;
+    bool mActivelyCollecting;
     // mScanInProgress should be false when we're collecting white objects.
     bool mScanInProgress;
     CycleCollectorResults mResults;
     TimeStamp mCollectionStart;
 
     CycleCollectedJSRuntime *mJSRuntime;
 
     ccPhase mIncrementalPhase;
@@ -2547,17 +2547,17 @@ nsCycleCollector::CollectReports(nsIHand
 
 
 ////////////////////////////////////////////////////////////////////////
 // Collector implementation
 ////////////////////////////////////////////////////////////////////////
 
 nsCycleCollector::nsCycleCollector() :
     MemoryMultiReporter("cycle-collector"),
-    mCollectionInProgress(false),
+    mActivelyCollecting(false),
     mScanInProgress(false),
     mJSRuntime(nullptr),
     mIncrementalPhase(IdlePhase),
     mThread(NS_GetCurrentThread()),
     mWhiteNodeCount(0),
     mBeforeUnlinkCB(nullptr),
     mForgetSkippableCB(nullptr),
     mUnmergedNeeded(0),
@@ -2674,17 +2674,16 @@ nsCycleCollector::FixGrayBits(bool aForc
     timeLog.Checkpoint("GC()");
 }
 
 void
 nsCycleCollector::CleanupAfterCollection()
 {
     MOZ_ASSERT(mIncrementalPhase == CleanupPhase);
     mGraph.Clear();
-    mCollectionInProgress = false;
 
 #ifdef XP_OS2
     // Now that the cycle collector has freed some memory, we can try to
     // force the C library to give back as much memory to the system as
     // possible.
     _heapmin();
 #endif
 
@@ -2720,27 +2719,29 @@ nsCycleCollector::ShutdownCollect()
 
 bool
 nsCycleCollector::Collect(ccType aCCType,
                           nsICycleCollectorListener *aManualListener)
 {
     CheckThreadSafety();
 
     // This can legitimately happen in a few cases. See bug 383651.
-    if (mCollectionInProgress) {
+    if (mActivelyCollecting) {
         return false;
     }
+    mActivelyCollecting = true;
 
     MOZ_ASSERT(mIncrementalPhase == IdlePhase);
 
     BeginCollection(aCCType, aManualListener);
     MarkRoots();
     ScanRoots();
     bool collectedAny = CollectWhite();
     CleanupAfterCollection();
+    mActivelyCollecting = false;
 
     MOZ_ASSERT(mIncrementalPhase == IdlePhase);
 
     return collectedAny;
 }
 
 // Don't merge too many times in a row, and do at least a minimum
 // number of unmerged CCs in a row.
@@ -2781,18 +2782,16 @@ void
 nsCycleCollector::BeginCollection(ccType aCCType,
                                   nsICycleCollectorListener *aManualListener)
 {
     TimeLog timeLog;
     MOZ_ASSERT(mIncrementalPhase == IdlePhase);
 
     mCollectionStart = TimeStamp::Now();
 
-    mCollectionInProgress = true;
-
     if (mJSRuntime) {
         mJSRuntime->BeginCycleCollectionCallback();
         timeLog.Checkpoint("BeginCycleCollectionCallback()");
     }
 
     bool isShutdown = (aCCType == ShutdownCC);
 
     // Set up the listener for this CC.