LibJS+LibGC: Run FinalizationRegistry cleanup host hook *after* GC

Before this change, it was possible for a second GC to get triggered
in the middle of a first GC, due to allocations happening in the
FinalizationRegistry cleanup host hook. To avoid this causing problems,
we add a "post-GC task" mechanism and use that to invoke the host hook
once all other GC activity is finished, and we've unset the "collecting
garbage" flag.

Note that the test included here only fails reliably when running with
the -g flag (collect garbage after each allocation).

Fixes #3051

Libraries/LibGC/Heap.cpp

@@ -240,23 +240,35 @@ AK::JsonObject Heap::dump_graph()
 void Heap::collect_garbage(CollectionType collection_type, bool print_report)
 {
     VERIFY(!m_collecting_garbage);
-    TemporaryChange change(m_collecting_garbage, true);
 
-    Core::ElapsedTimer collection_measurement_timer;
+    {
-    if (print_report)
+        TemporaryChange change(m_collecting_garbage, true);
-        collection_measurement_timer.start();
 
-    if (collection_type == CollectionType::CollectGarbage) {
+        Core::ElapsedTimer collection_measurement_timer;
-        if (m_gc_deferrals) {
+        if (print_report)
-            m_should_gc_when_deferral_ends = true;
+            collection_measurement_timer.start();
-            return;
+
+        if (collection_type == CollectionType::CollectGarbage) {
+            if (m_gc_deferrals) {
+                m_should_gc_when_deferral_ends = true;
+                return;
+            }
+            HashMap<Cell*, HeapRoot> roots;
+            gather_roots(roots);
+            mark_live_cells(roots);
         }
-        HashMap<Cell*, HeapRoot> roots;
+        finalize_unmarked_cells();
-        gather_roots(roots);
+        sweep_dead_cells(print_report, collection_measurement_timer);
-        mark_live_cells(roots);
     }
-    finalize_unmarked_cells();
+
-    sweep_dead_cells(print_report, collection_measurement_timer);
+    auto tasks = move(m_post_gc_tasks);
+    for (auto& task : tasks)
+        task();
+}
+
+void Heap::enqueue_post_gc_task(AK::Function<void()> task)
+{
+    m_post_gc_tasks.append(move(task));
 }
 
 void Heap::gather_roots(HashMap<Cell*, HeapRoot>& roots)

Libraries/LibGC/Heap.h

@@ -76,6 +76,8 @@ public:
 
     bool is_gc_deferred() const { return m_gc_deferrals > 0; }
 
+    void enqueue_post_gc_task(AK::Function<void()>);
+
 private:
     friend class MarkingVisitor;
     friend class GraphConstructorVisitor;
@@ -151,6 +153,8 @@ private:
     bool m_collecting_garbage { false };
     StackInfo m_stack_info;
     AK::Function<void(HashMap<Cell*, GC::HeapRoot>&)> m_gather_embedder_roots;
+
+    Vector<AK::Function<void()>> m_post_gc_tasks;
 } SWIFT_IMMORTAL_REFERENCE;
 
 inline void Heap::did_create_root(Badge<RootImpl>, RootImpl& impl)

Libraries/LibJS/Runtime/FinalizationRegistry.cpp

@@ -57,8 +57,13 @@ void FinalizationRegistry::remove_dead_cells(Badge<GC::Heap>)
         any_cells_were_removed = true;
         break;
     }
-    if (any_cells_were_removed)
+    if (any_cells_were_removed) {
-        vm().host_enqueue_finalization_registry_cleanup_job(*this);
+        // NOTE: We make a GC::Root here to ensure that the FinalizationRegistry stays alive
+        //       even if a subsequent GC is triggered before the callback has a chance to run.
+        heap().enqueue_post_gc_task([that = GC::make_root(this)]() {
+            that->vm().host_enqueue_finalization_registry_cleanup_job(*that);
+        });
+    }
 }
 
 // 9.13 CleanupFinalizationRegistry ( finalizationRegistry ), https://tc39.es/ecma262/#sec-cleanup-finalization-registry

Tests/LibWeb/Crash/JS/finalization-registry-basic.html

@@ -0,0 +1,19 @@
+<script src="../include.js"></script>
+<script>
+
+// NOTE: This test is only reliable when GC'ing after each allocation.
+
+const registry = new FinalizationRegistry((heldValue) => {
+    console.log(heldValue);
+});
+
+const makeGarbage = () => {
+    registry.register(new String("ok"), "hello");
+};
+makeGarbage();
+
+if (window.internals !== undefined)
+    internals.gc();
+
+println("PASS");
+</script>