Bug 1596756 - Don't queue finalization groups for cleanup during shutdown r=sfink

It doesn't make sense to queue callbacks at this point because they will never run, and without this we get leaks at shutdown.  I also refactored GCRuntime::sweepFinalizationGroups a little.

Differential Revision: https://phabricator.services.mozilla.com/D59897

--HG--
extra : moz-landing-system : lando

js/src/gc/FinalizationGroup.cpp

@@ -50,7 +50,18 @@ void GCRuntime::markFinalizationGroupData(JSTracer* trc) {
   }
 }
 
-void GCRuntime::sweepFinalizationGroups(Zone* zone) {
+static FinalizationRecordObject* UnwrapFinalizationRecord(JSObject* obj) {
+  obj = UncheckedUnwrapWithoutExpose(obj);
+  if (!obj->is<FinalizationRecordObject>()) {
+    MOZ_ASSERT(JS_IsDeadWrapper(obj));
+    // CCWs between the compartments have been nuked. The
+    // FinalizationGroup's callback doesn't run in this case.
+    return nullptr;
+  }
+  return &obj->as<FinalizationRecordObject>();
+}
+
+void GCRuntime::sweepFinalizationGroups(Zone* zone, bool isShuttingDown) {
   // Queue holdings for cleanup for any entries whose target is dying and remove
   // them from the map. Sweep remaining unregister tokens.
 
@@ -60,18 +71,12 @@ void GCRuntime::sweepFinalizationGroups(Zone* zone) {
     if (IsAboutToBeFinalized(&e.front().mutableKey())) {
       // Queue holdings for targets that are dying.
       for (JSObject* obj : records) {
-        obj = UncheckedUnwrapWithoutExpose(obj);
-        if (!obj->is<FinalizationRecordObject>()) {
-          MOZ_ASSERT(JS_IsDeadWrapper(obj));
-          // CCWs between the compartments have been nuked. The
-          // FinalizationGroup's callback doesn't run in this case.
-          continue;
-        }
-        auto record = &obj->as<FinalizationRecordObject>();
-        FinalizationGroupObject* group = record->group();
-        if (group) {
-          group->queueRecordToBeCleanedUp(record);
-          queueFinalizationGroupForCleanup(group);
+        if (FinalizationRecordObject* record = UnwrapFinalizationRecord(obj)) {
+          FinalizationGroupObject* group = record->group();
+          if (group && !isShuttingDown) {
+            group->queueRecordToBeCleanedUp(record);
+            queueFinalizationGroupForCleanup(group);
+          }
         }
       }
       e.removeFront();
@@ -80,15 +85,8 @@ void GCRuntime::sweepFinalizationGroups(Zone* zone) {
       records.sweep();
       // Remove records that have been unregistered.
       records.eraseIf([](JSObject* obj) {
-        obj = UncheckedUnwrapWithoutExpose(obj);
-        if (!obj->is<FinalizationRecordObject>()) {
-          MOZ_ASSERT(JS_IsDeadWrapper(obj));
-          // CCWs between the compartments have been nuked. The
-          // FinalizationGroup's callback doesn't run in this case.
-          return true;
-        }
-        auto record = &obj->as<FinalizationRecordObject>();
-        return !record->group();
+        FinalizationRecordObject* record = UnwrapFinalizationRecord(obj);
+        return !record || !record->group();
       });
     }
   }

js/src/gc/GC.cpp

@@ -4921,12 +4921,18 @@ static void SweepUniqueIds(GCParallelTask* task) {
   }
 }
 
+static bool IsShutdownGC(JS::GCReason reason) {
+  return reason == JS::GCReason::WORKER_SHUTDOWN ||
+         reason == JS::GCReason::SHUTDOWN_CC ||
+         reason == JS::GCReason::DESTROY_RUNTIME;
+}
+
 void GCRuntime::sweepFinalizationGroupsOnMainThread() {
   // This calls back into the browser which expects to be called from the main
   // thread.
   gcstats::AutoPhase ap(stats(), gcstats::PhaseKind::SWEEP_FINALIZATION_GROUPS);
   for (SweepGroupZonesIter zone(this); !zone.done(); zone.next()) {
-    sweepFinalizationGroups(zone);
+    sweepFinalizationGroups(zone, IsShutdownGC(initialReason));
   }
 }
 
@@ -6402,11 +6408,6 @@ AutoDisableBarriers::~AutoDisableBarriers() {
   }
 }
 
-static bool IsShutdownGC(JS::GCReason reason) {
-  return reason == JS::GCReason::SHUTDOWN_CC ||
-         reason == JS::GCReason::DESTROY_RUNTIME;
-}
-
 static bool ShouldCleanUpEverything(JS::GCReason reason,
                                     JSGCInvocationKind gckind) {
   // During shutdown, we must clean everything up, for the sake of leak

js/src/gc/GCRuntime.h

@@ -701,7 +701,7 @@ class GCRuntime {
   void sweepDebuggerOnMainThread(JSFreeOp* fop);
   void sweepJitDataOnMainThread(JSFreeOp* fop);
   void sweepFinalizationGroupsOnMainThread();
-  void sweepFinalizationGroups(Zone* zone);
+  void sweepFinalizationGroups(Zone* zone, bool isShuttingDown = false);
   void queueFinalizationGroupForCleanup(FinalizationGroupObject* group);
   void sweepWeakRefs(Zone* zone);
   friend void SweepWeakRefs(GCParallelTask* task);