Bug 1108836 - Fix race condition starting background sweeping r=terrence

js/src/gc/Zone.cpp

@@ -289,7 +289,7 @@ ZoneList::ZoneList()
 ZoneList::ZoneList(Zone *zone)
   : head(zone), tail(zone)
 {
-    MOZ_ASSERT(!zone->isOnList());
+    MOZ_RELEASE_ASSERT(!zone->isOnList());
     zone->listNext_ = nullptr;
 }
 
@@ -333,7 +333,6 @@ ZoneList::front() const
 void
 ZoneList::append(Zone *zone)
 {
-    MOZ_ASSERT(!zone->isOnList());
     ZoneList singleZone(zone);
     transferFrom(singleZone);
 }

js/src/jit-test/tests/gc/bug-1108836.js

@@ -0,0 +1,9 @@
+const root = newGlobal();
+var g = newGlobal();
+for (var indexI = 0; indexI <= 65535; indexI++) {
+    eval("/*var " + String.fromCharCode(indexI) + "xx = 1*/");
+}
+for (var i = 0; i < 100; ++i) {
+    gc();
+    gcslice(1000000);
+}

js/src/jsgc.cpp

@@ -3641,8 +3641,6 @@ GCHelperState::maybeStartBackgroundSweep(const AutoLockGC &lock)
 {
     MOZ_ASSERT(CanUseExtraThreads());
 
-    sweepFlag = true;
-    shrinkFlag = false;
     if (state() == IDLE)
         startBackgroundThread(SWEEPING);
 }
@@ -3653,7 +3651,6 @@ GCHelperState::startBackgroundShrink(const AutoLockGC &lock)
     MOZ_ASSERT(CanUseExtraThreads());
     switch (state()) {
       case IDLE:
-        MOZ_ASSERT(!sweepFlag);
         shrinkFlag = true;
         startBackgroundThread(SWEEPING);
         break;
@@ -3678,30 +3675,26 @@ GCHelperState::waitBackgroundSweepEnd()
 void
 GCHelperState::doSweep(AutoLockGC &lock)
 {
-    while (sweepFlag) {
-        sweepFlag = false;
-        ZoneList zones;
-        zones.transferFrom(rt->gc.backgroundSweepZones);
-        LifoAlloc freeLifoAlloc(JSRuntime::TEMP_LIFO_ALLOC_PRIMARY_CHUNK_SIZE);
-        freeLifoAlloc.transferFrom(&rt->gc.freeLifoAlloc);
-        AutoUnlockGC unlock(lock);
+    // The main thread may call queueZonesForBackgroundSweep() or
+    // ShrinkGCBuffers() while this is running so we must check there is no more
+    // work to do before exiting.
 
-        rt->gc.sweepBackgroundThings(zones, BackgroundThread);
-        freeLifoAlloc.freeAll();
-    }
+    do {
+        while (!rt->gc.backgroundSweepZones.isEmpty()) {
+            ZoneList zones;
+            zones.transferFrom(rt->gc.backgroundSweepZones);
+            LifoAlloc freeLifoAlloc(JSRuntime::TEMP_LIFO_ALLOC_PRIMARY_CHUNK_SIZE);
+            freeLifoAlloc.transferFrom(&rt->gc.freeLifoAlloc);
+            AutoUnlockGC unlock(lock);
 
-    bool shrinking = shrinkFlag;
-    rt->gc.expireChunksAndArenas(shrinking, lock);
+            rt->gc.sweepBackgroundThings(zones, BackgroundThread);
+            freeLifoAlloc.freeAll();
+        }
 
-    /*
-     * The main thread may have called ShrinkGCBuffers while
-     * ExpireChunksAndArenas(rt, false) was running, so we recheck the flag
-     * afterwards.
-     */
-    if (!shrinking && shrinkFlag) {
+        bool shrinking = shrinkFlag;
         shrinkFlag = false;
-        rt->gc.expireChunksAndArenas(true, lock);
-    }
+        rt->gc.expireChunksAndArenas(shrinking, lock);
+    } while (!rt->gc.backgroundSweepZones.isEmpty() || shrinkFlag);
 }
 
 bool

js/src/jsgc.h

@@ -1008,8 +1008,7 @@ class GCHelperState
     State state();
     void setState(State state);
 
-    bool              sweepFlag;
-    bool              shrinkFlag;
+    bool shrinkFlag;
 
     friend class js::gc::ArenaLists;
 
@@ -1028,7 +1027,6 @@ class GCHelperState
         done(nullptr),
         state_(IDLE),
         thread(nullptr),
-        sweepFlag(false),
         shrinkFlag(false)
     { }