diff --git a/src/memory_tracker-inl.h b/src/memory_tracker-inl.h index 3c82983ce01e..2e32e1136986 100644 --- a/src/memory_tracker-inl.h +++ b/src/memory_tracker-inl.h @@ -300,7 +300,7 @@ void MemoryTracker::Track(const CppgcMixin* retainer, const char* edge_name) { auto it = seen_.find(retainer); if (it != seen_.end()) { if (CurrentNode() != nullptr) { - AddEdge(CurrentNode(), it->second, edge_name); + AddEdge(CurrentNode(), it->second->JSWrapperNode(), edge_name); } return; // It has already been tracked, no need to call MemoryInfo again } @@ -357,6 +357,11 @@ inline void MemoryTracker::TraitTrackInline(const T& retainer, -(static_cast(MemoryRetainerTraits::SelfSize(retainer)))); } +MemoryTracker::MemoryTracker(v8::Isolate* isolate, v8::EmbedderGraph* graph) + : isolate_(isolate), graph_(graph) {} + +MemoryTracker::~MemoryTracker() = default; + v8::EmbedderGraph::Node* MemoryTracker::CurrentNode() const { if (node_stack_.empty()) return nullptr; MemoryRetainerNode* n = node_stack_.top(); @@ -373,7 +378,8 @@ MemoryRetainerNode* MemoryTracker::AddNode(const CppgcMixin* retainer, return it->second; } - MemoryRetainerNode* n = new MemoryRetainerNode(this, retainer); + cppgc_nodes_.push_back(std::make_unique(this, retainer)); + MemoryRetainerNode* n = cppgc_nodes_.back().get(); seen_[retainer] = n; if (CurrentNode() != nullptr) { AddEdge(CurrentNode(), n->JSWrapperNode(), edge_name); diff --git a/src/memory_tracker.h b/src/memory_tracker.h index d7893f10b3af..5117cc532711 100644 --- a/src/memory_tracker.h +++ b/src/memory_tracker.h @@ -8,10 +8,12 @@ #include #include +#include #include #include #include #include +#include namespace v8 { class BackingStore; @@ -294,9 +296,8 @@ class MemoryTracker { inline v8::EmbedderGraph* graph() { return graph_; } inline v8::Isolate* isolate() { return isolate_; } - inline explicit MemoryTracker(v8::Isolate* isolate, - v8::EmbedderGraph* graph) - : isolate_(isolate), graph_(graph) {} + inline explicit MemoryTracker(v8::Isolate* isolate, v8::EmbedderGraph* graph); + inline ~MemoryTracker(); // Can be passed to Track() if it is not desirable // to create a strong edge between nodes, i.e. when @@ -334,6 +335,8 @@ class MemoryTracker { v8::EmbedderGraph* graph_; std::stack node_stack_; NodeMap seen_; + // Placeholder nodes for cppgc wrappers; the graph only owns their JS nodes. + std::vector> cppgc_nodes_; }; } // namespace node diff --git a/test/cctest/test_environment.cc b/test/cctest/test_environment.cc index 36fbc0e79d46..c0e9138dfe14 100644 --- a/test/cctest/test_environment.cc +++ b/test/cctest/test_environment.cc @@ -328,6 +328,19 @@ TEST_F(EnvironmentTest, MultipleEnvironmentsPerIsolate) { EXPECT_TRUE(called_cb_2); } +TEST_F(EnvironmentTest, HeapSnapshotWithCppgcWrappersDoesNotLeak) { + const v8::HandleScope handle_scope(isolate_); + const Argv argv; + Env env{handle_scope, argv}; + node::LoadEnvironment(*env, + "const vm = require('vm');" + "globalThis.script = new vm.Script('1');" + "globalThis.context = vm.createContext();") + .ToLocalChecked(); + node::heap::HeapSnapshotPointer snapshot{ + isolate_->GetHeapProfiler()->TakeHeapSnapshot()}; +} + TEST_F(EnvironmentTest, NoEnvironmentSanity) { const v8::HandleScope handle_scope(isolate_); v8::Local context = v8::Context::New(isolate_);