Bug 1081952 - Explicitly set the background finalize state as the last step. r=terrence
authorEmanuel Hoogeveen <emanuel.hoogeveen@gmail.com>
Mon, 13 Oct 2014 10:17:00 +0200
changeset 210328 f9faedda0179e32f5b103920b2ba154fc78118bb
parent 210327 3193763d0a3bbe6bfca4a07f73802320e3b8ca07
child 210329 f3f97d35b18016460c99c94b460c80c28341545c
push id1
push userroot
push dateMon, 20 Oct 2014 17:29:22 +0000
reviewersterrence
bugs1081952
milestone36.0a1
Bug 1081952 - Explicitly set the background finalize state as the last step. r=terrence
js/src/jsgc.cpp
--- a/js/src/jsgc.cpp
+++ b/js/src/jsgc.cpp
@@ -2567,24 +2567,32 @@ ArenaLists::backgroundFinalize(FreeOp *f
     // arenas may be allocated before background finalization finishes; now that
     // finalization is complete, we want to merge these lists back together.
     ArenaLists *lists = &zone->allocator.arenas;
     ArenaList *al = &lists->arenaLists[thingKind];
 
     // Flatten |finalizedSorted| into a regular ArenaList.
     ArenaList finalized = finalizedSorted.toArenaList();
 
-    AutoLockGC lock(fop->runtime());
-    MOZ_ASSERT(lists->backgroundFinalizeState[thingKind] == BFS_RUN);
-
-    // Join |al| and |finalized| into a single list.
-    *al = finalized.insertListWithCursorAtEnd(*al);
+    // We must take the GC lock to be able to safely modify the ArenaList;
+    // however, this does not by itself make the changes visible to all threads,
+    // as not all threads take the GC lock to read the ArenaLists.
+    // That safety is provided by the ReleaseAcquire memory ordering of the
+    // background finalize state, which we explicitly set as the final step.
+    {
+        AutoLockGC lock(fop->runtime());
+        MOZ_ASSERT(lists->backgroundFinalizeState[thingKind] == BFS_RUN);
+
+        // Join |al| and |finalized| into a single list.
+        *al = finalized.insertListWithCursorAtEnd(*al);
+
+        lists->arenaListsToSweep[thingKind] = nullptr;
+    }
 
     lists->backgroundFinalizeState[thingKind] = BFS_DONE;
-    lists->arenaListsToSweep[thingKind] = nullptr;
 }
 
 void
 ArenaLists::queueObjectsForSweep(FreeOp *fop)
 {
     gcstats::AutoPhase ap(fop->runtime()->gc.stats, gcstats::PHASE_SWEEP_OBJECT);
 
     finalizeNow(fop, FINALIZE_OBJECT0);