Revert "LibJS: Remove "uprooting" mechanism from garbage collector"

This reverts commit 6232ad3a0d44a3833c476d37bf4084d39a10a74b.

Unfortunately this introduced some flakiness on CI, so it wasn't
quite this simple.

Tests/LibJS/test-js.cpp

@@ -49,6 +49,34 @@ TESTJS_GLOBAL_FUNCTION(get_weak_map_size, getWeakMapSize)
     return JS::Value(weak_map.values().size());
 }
 
+TESTJS_GLOBAL_FUNCTION(mark_as_garbage, markAsGarbage)
+{
+    auto argument = vm.argument(0);
+    if (!argument.is_string())
+        return vm.throw_completion<JS::TypeError>(JS::ErrorType::NotAString, TRY_OR_THROW_OOM(vm, argument.to_string_without_side_effects()));
+
+    auto& variable_name = argument.as_string();
+
+    // In native functions we don't have a lexical environment so get the outer via the execution stack.
+    auto outer_environment = vm.execution_context_stack().last_matching([&](auto& execution_context) {
+        return execution_context->lexical_environment != nullptr;
+    });
+    if (!outer_environment.has_value())
+        return vm.throw_completion<JS::ReferenceError>(JS::ErrorType::UnknownIdentifier, TRY(variable_name.deprecated_string()));
+
+    auto reference = TRY(vm.resolve_binding(TRY(variable_name.deprecated_string()), outer_environment.value()->lexical_environment));
+
+    auto value = TRY(reference.get_value(vm));
+
+    if (!can_be_held_weakly(value))
+        return vm.throw_completion<JS::TypeError>(JS::ErrorType::CannotBeHeldWeakly, DeprecatedString::formatted("Variable with name {}", TRY(variable_name.deprecated_string())));
+
+    vm.heap().uproot_cell(&value.as_cell());
+    TRY(reference.delete_(vm));
+
+    return JS::js_undefined();
+}
+
 TESTJS_GLOBAL_FUNCTION(detach_array_buffer, detachArrayBuffer)
 {
     auto array_buffer = vm.argument(0);

Userland/Libraries/LibJS/Heap/Heap.cpp

@@ -276,6 +276,11 @@ void Heap::mark_live_cells(HashTable<Cell*> const& roots)
         bytecode_interpreter->visit_edges(visitor);
 
     visitor.mark_all_live_cells();
+
+    for (auto& inverse_root : m_uprooted_cells)
+        inverse_root->set_marked(false);
+
+    m_uprooted_cells.clear();
 }
 
 bool Heap::cell_must_survive_garbage_collection(Cell const& cell)
@@ -422,6 +427,11 @@ void Heap::undefer_gc(Badge<DeferGC>)
     }
 }
 
+void Heap::uproot_cell(Cell* cell)
+{
+    m_uprooted_cells.append(cell);
+}
+
 void register_safe_function_closure(void* base, size_t size)
 {
     if (!s_custom_ranges_for_conservative_scan) {

Userland/Libraries/LibJS/Heap/Heap.h

@@ -76,6 +76,8 @@ public:
 
     BlockAllocator& block_allocator() { return m_block_allocator; }
 
+    void uproot_cell(Cell* cell);
+
 private:
     static bool cell_must_survive_garbage_collection(Cell const&);
 
@@ -110,6 +112,8 @@ private:
     MarkedVectorBase::List m_marked_vectors;
     WeakContainer::List m_weak_containers;
 
+    Vector<GCPtr<Cell>> m_uprooted_cells;
+
     BlockAllocator m_block_allocator;
 
     size_t m_gc_deferrals { 0 };

Userland/Libraries/LibJS/Tests/builtins/FinalizationRegistry/FinalizationRegistry.prototype.cleanupSome.js

@@ -3,7 +3,9 @@ test("length is 0", () => {
 });
 
 function registerInDifferentScope(registry) {
-    registry.register({}, {});
+    const target = {};
+    registry.register(target, {});
+    return target;
 }
 
 test("basic functionality", () => {
@@ -18,7 +20,8 @@ test("basic functionality", () => {
 
     expect(count).toBe(0);
 
-    registerInDifferentScope(registry);
+    const target = registerInDifferentScope(registry);
+    markAsGarbage("target");
     gc();
 
     registry.cleanupSome(increment);

Userland/Libraries/LibJS/Tests/builtins/WeakMap/WeakMap.prototype.set.js

@@ -1,9 +1,3 @@
-function registerInDifferentScope(registry) {
-    const target = {};
-    registry.register(target, {});
-    eval("");
-}
-
 test("basic functionality", () => {
     expect(WeakMap.prototype.set).toHaveLength(2);
 
@@ -27,26 +21,25 @@ test("invalid values", () => {
     });
 });
 
-function setObjectKey(weakMap) {
-    expect(weakMap.set({ e: 3 }, 1)).toBe(weakMap);
-}
-
-function setSymbolKey(weakMap) {
-    expect(weakMap.set(Symbol("foo"), "bar")).toBe(weakMap);
-}
-
 test("automatic removal of garbage-collected values", () => {
     const weakMap = new WeakMap();
+    const objectKey = { e: 3 };
 
-    setObjectKey(weakMap);
+    expect(weakMap.set(objectKey, 1)).toBe(weakMap);
     expect(getWeakMapSize(weakMap)).toBe(1);
 
+    markAsGarbage("objectKey");
     gc();
+
     expect(getWeakMapSize(weakMap)).toBe(0);
 
-    setSymbolKey(weakMap);
+    const symbolKey = Symbol("foo");
+
+    expect(weakMap.set(symbolKey, "bar")).toBe(weakMap);
     expect(getWeakMapSize(weakMap)).toBe(1);
 
+    markAsGarbage("symbolKey");
     gc();
+
     expect(getWeakMapSize(weakMap)).toBe(0);
 });

Userland/Libraries/LibJS/Tests/builtins/WeakSet/WeakSet.prototype.add.js

@@ -16,26 +16,25 @@ test("invalid values", () => {
     });
 });
 
-function addObjectItem(weakSet) {
-    weakSet.add({ a: 1 });
-}
-
-function addSymbolItem(weakSet) {
-    weakSet.add(Symbol("foo"));
-}
-
 test("automatic removal of garbage-collected values", () => {
     const weakSet = new WeakSet();
+    const objectItem = { a: 1 };
 
-    addObjectItem(weakSet);
+    expect(weakSet.add(objectItem)).toBe(weakSet);
     expect(getWeakSetSize(weakSet)).toBe(1);
 
+    markAsGarbage("objectItem");
     gc();
+
     expect(getWeakSetSize(weakSet)).toBe(0);
 
-    addSymbolItem(weakSet);
+    const symbolItem = Symbol("foo");
+
+    expect(weakSet.add(symbolItem)).toBe(weakSet);
     expect(getWeakSetSize(weakSet)).toBe(1);
 
+    markAsGarbage("symbolItem");
     gc();
+
     expect(getWeakSetSize(weakSet)).toBe(0);
 });