Jelajahi Sumber

test-js: Add a mark_as_garbage method to force GC to collect that object

This should fix the flaky tests of test-js.
It also fixes the tests when running with the -g flag since the values
will not be garbage collected too soon.
davidot 3 tahun lalu
induk
melakukan
43b17f27a3

+ 36 - 0
Tests/LibJS/test-js.cpp

@@ -58,6 +58,42 @@ 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()) {
+        vm.throw_exception<JS::TypeError>(global_object, JS::ErrorType::NotAString, argument.to_string_without_side_effects());
+        return {};
+    }
+
+    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()) {
+        vm.throw_exception<JS::ReferenceError>(global_object, JS::ErrorType::UnknownIdentifier, variable_name.string());
+        return {};
+    }
+
+    auto variable = outer_environment.value()->lexical_environment->get_from_environment(variable_name.string());
+    if (!variable.has_value()) {
+        vm.throw_exception<JS::ReferenceError>(global_object, JS::ErrorType::UnknownIdentifier, variable_name.string());
+        return {};
+    }
+
+    if (!variable->value.is_object()) {
+        vm.throw_exception<JS::TypeError>(global_object, JS::ErrorType::NotAnObject, String::formatted("Variable with name {}", variable_name.string()));
+        return {};
+    }
+
+    vm.heap().uproot_cell(&variable->value.as_object());
+    outer_environment.value()->lexical_environment->delete_from_environment(variable_name.string());
+
+    return JS::js_undefined();
+}
+
 TESTJS_RUN_FILE_FUNCTION(const String& test_file, JS::Interpreter&)
 {
     if (!test262_parser_tests)

+ 11 - 0
Userland/Libraries/LibJS/Heap/Heap.cpp

@@ -191,9 +191,15 @@ public:
 void Heap::mark_live_cells(const HashTable<Cell*>& roots)
 {
     dbgln_if(HEAP_DEBUG, "mark_live_cells:");
+
     MarkingVisitor visitor;
     for (auto* root : roots)
         visitor.visit(root);
+
+    for (auto& inverse_root : m_uprooted_cells)
+        inverse_root->set_marked(false);
+
+    m_uprooted_cells.clear();
 }
 
 void Heap::sweep_dead_cells(bool print_report, const Core::ElapsedTimer& measurement_timer)
@@ -327,4 +333,9 @@ void Heap::undefer_gc(Badge<DeferGC>)
     }
 }
 
+void Heap::uproot_cell(Cell* cell)
+{
+    m_uprooted_cells.append(cell);
+}
+
 }

+ 4 - 0
Userland/Libraries/LibJS/Heap/Heap.h

@@ -83,6 +83,8 @@ public:
 
     BlockAllocator& block_allocator() { return m_block_allocator; }
 
+    void uproot_cell(Cell* cell);
+
 private:
     Cell* allocate_cell(size_t);
 
@@ -117,6 +119,8 @@ private:
 
     WeakContainer::List m_weak_containers;
 
+    Vector<Cell*> m_uprooted_cells;
+
     BlockAllocator m_block_allocator;
 
     size_t m_gc_deferrals { 0 };

+ 6 - 4
Userland/Libraries/LibJS/Tests/builtins/FinalizationRegistry/FinalizationRegistry.prototype.cleanupSome.js

@@ -3,11 +3,12 @@ test("length is 0", () => {
 });
 
 function registerInDifferentScope(registry) {
-    registry.register({}, {});
+    const target = {};
+    registry.register(target, {});
+    return target;
 }
 
-// Flaky test, investigate and fix :^)
-test.skip("basic functionality", () => {
+test("basic functionality", () => {
     var registry = new FinalizationRegistry(() => {});
 
     var count = 0;
@@ -19,7 +20,8 @@ test.skip("basic functionality", () => {
 
     expect(count).toBe(0);
 
-    registerInDifferentScope(registry);
+    const target = registerInDifferentScope(registry);
+    markAsGarbage("target");
     gc();
 
     registry.cleanupSome(increment);

+ 7 - 4
Userland/Libraries/LibJS/Tests/builtins/WeakMap/WeakMap.prototype.set.js

@@ -21,10 +21,13 @@ test("invalid values", () => {
 
 test("automatic removal of garbage-collected values", () => {
     const weakMap = new WeakMap();
-    {
-        expect(weakMap.set({ a: 1 }, 1)).toBe(weakMap);
-        expect(getWeakMapSize(weakMap)).toBe(1);
-    }
+    const key = { e: 3 };
+
+    expect(weakMap.set(key, 1)).toBe(weakMap);
+    expect(getWeakMapSize(weakMap)).toBe(1);
+
+    markAsGarbage("key");
     gc();
+
     expect(getWeakMapSize(weakMap)).toBe(0);
 });

+ 7 - 4
Userland/Libraries/LibJS/Tests/builtins/WeakSet/WeakSet.prototype.add.js

@@ -17,10 +17,13 @@ test("invalid values", () => {
 
 test("automatic removal of garbage-collected values", () => {
     const weakSet = new WeakSet();
-    {
-        expect(weakSet.add({ a: 1 })).toBe(weakSet);
-        expect(getWeakSetSize(weakSet)).toBe(1);
-    }
+    const item = { a: 1 };
+
+    expect(weakSet.add(item)).toBe(weakSet);
+    expect(getWeakSetSize(weakSet)).toBe(1);
+
+    markAsGarbage("item");
     gc();
+
     expect(getWeakSetSize(weakSet)).toBe(0);
 });