Переглянути джерело

LibJS: Only consider VM-accessible execution contexts as strong roots

Partially reverts 3dc5f467a8200b7c4043cf01d78b08745ddf8528 to fix
GC memory leak that happens because we treated all execution contexts
as strong roots.
Aliaksandr Kalenik 1 рік тому
батько
коміт
b108d51c5b

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

@@ -450,9 +450,6 @@ void Heap::mark_live_cells(HashMap<Cell*, HeapRoot> const& roots)
 
     MarkingVisitor visitor(*this, roots);
 
-    for (auto& execution_context : m_execution_contexts)
-        execution_context.visit_edges(visitor);
-
     vm().bytecode_interpreter().visit_edges(visitor);
 
     visitor.mark_all_live_cells();

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

@@ -148,7 +148,6 @@ private:
     HandleImpl::List m_handles;
     MarkedVectorBase::List m_marked_vectors;
     WeakContainer::List m_weak_containers;
-    ExecutionContext::List m_execution_contexts;
 
     Vector<GCPtr<Cell>> m_uprooted_cells;
 
@@ -196,16 +195,4 @@ inline void Heap::did_destroy_weak_container(Badge<WeakContainer>, WeakContainer
     m_weak_containers.remove(set);
 }
 
-inline void Heap::did_create_execution_context(Badge<ExecutionContext>, ExecutionContext& set)
-{
-    VERIFY(!m_execution_contexts.contains(set));
-    m_execution_contexts.append(set);
-}
-
-inline void Heap::did_destroy_execution_context(Badge<ExecutionContext>, ExecutionContext& set)
-{
-    VERIFY(m_execution_contexts.contains(set));
-    m_execution_contexts.remove(set);
-}
-
 }

+ 2 - 0
Userland/Libraries/LibJS/Runtime/AsyncFunctionDriverWrapper.cpp

@@ -192,6 +192,8 @@ void AsyncFunctionDriverWrapper::visit_edges(Cell::Visitor& visitor)
     visitor.visit(m_top_level_promise);
     if (m_current_promise)
         visitor.visit(m_current_promise);
+    if (m_suspended_execution_context)
+        m_suspended_execution_context->visit_edges(visitor);
 }
 
 }

+ 1 - 0
Userland/Libraries/LibJS/Runtime/AsyncGenerator.cpp

@@ -48,6 +48,7 @@ void AsyncGenerator::visit_edges(Cell::Visitor& visitor)
     if (m_frame)
         m_frame->visit_edges(visitor);
     visitor.visit(m_current_promise);
+    m_async_generator_context->visit_edges(visitor);
 }
 
 // 27.6.3.4 AsyncGeneratorEnqueue ( generator, completion, promiseCapability ), https://tc39.es/ecma262/#sec-asyncgeneratorenqueue

+ 0 - 2
Userland/Libraries/LibJS/Runtime/ExecutionContext.cpp

@@ -21,12 +21,10 @@ NonnullOwnPtr<ExecutionContext> ExecutionContext::create(Heap& heap)
 ExecutionContext::ExecutionContext(Heap& heap)
     : m_heap(heap)
 {
-    m_heap.did_create_execution_context({}, *this);
 }
 
 ExecutionContext::~ExecutionContext()
 {
-    m_heap.did_destroy_execution_context({}, *this);
 }
 
 NonnullOwnPtr<ExecutionContext> ExecutionContext::copy() const

+ 1 - 0
Userland/Libraries/LibJS/Runtime/GeneratorObject.cpp

@@ -51,6 +51,7 @@ void GeneratorObject::visit_edges(Cell::Visitor& visitor)
     visitor.visit(m_previous_value);
     if (m_frame)
         m_frame->visit_edges(visitor);
+    m_execution_context->visit_edges(visitor);
 }
 
 // 27.5.3.2 GeneratorValidate ( generator, generatorBrand ), https://tc39.es/ecma262/#sec-generatorvalidate

+ 26 - 0
Userland/Libraries/LibJS/Runtime/VM.cpp

@@ -153,6 +153,20 @@ Bytecode::Interpreter& VM::bytecode_interpreter()
     return *m_bytecode_interpreter;
 }
 
+struct ExecutionContextRootsCollector : public Cell::Visitor {
+    virtual void visit_impl(Cell& cell) override
+    {
+        roots.set(&cell);
+    }
+
+    virtual void visit_possible_values(ReadonlyBytes) override
+    {
+        VERIFY_NOT_REACHED();
+    }
+
+    HashTable<Cell*> roots;
+};
+
 void VM::gather_roots(HashMap<Cell*, HeapRoot>& roots)
 {
     roots.set(m_empty_string, HeapRoot { .type = HeapRoot::Type::VM });
@@ -169,6 +183,18 @@ void VM::gather_roots(HashMap<Cell*, HeapRoot>& roots)
 
     for (auto finalization_registry : m_finalization_registry_cleanup_jobs)
         roots.set(finalization_registry, HeapRoot { .type = HeapRoot::Type::VM });
+
+    auto gather_roots_from_execution_context_stack = [&roots](Vector<ExecutionContext*> const& stack) {
+        for (auto const& execution_context : stack) {
+            ExecutionContextRootsCollector visitor;
+            execution_context->visit_edges(visitor);
+            for (auto* cell : visitor.roots)
+                roots.set(cell, HeapRoot { .type = HeapRoot::Type::VM });
+        }
+    };
+    gather_roots_from_execution_context_stack(m_execution_context_stack);
+    for (auto& saved_stack : m_saved_execution_context_stacks)
+        gather_roots_from_execution_context_stack(saved_stack);
 }
 
 ThrowCompletionOr<Value> VM::named_evaluation_if_anonymous_function(ASTNode const& expression, DeprecatedFlyString const& name)

+ 1 - 0
Userland/Libraries/LibJS/SourceTextModule.cpp

@@ -120,6 +120,7 @@ void SourceTextModule::visit_edges(Cell::Visitor& visitor)
 {
     Base::visit_edges(visitor);
     visitor.visit(m_import_meta);
+    m_execution_context->visit_edges(visitor);
 }
 
 // 16.2.1.6.1 ParseModule ( sourceText, realm, hostDefined ), https://tc39.es/ecma262/#sec-parsemodule

+ 1 - 0
Userland/Libraries/LibWeb/HTML/Scripting/Environments.cpp

@@ -45,6 +45,7 @@ void EnvironmentSettingsObject::visit_edges(Cell::Visitor& visitor)
     visitor.visit(target_browsing_context);
     visitor.visit(m_module_map);
     visitor.ignore(m_outstanding_rejected_promises_weak_set);
+    m_realm_execution_context->visit_edges(visitor);
 }
 
 JS::ExecutionContext& EnvironmentSettingsObject::realm_execution_context()