src: do not track weak BaseObjects as childrens of Realms

src/base_object.cc

@@ -174,10 +174,18 @@ void BaseObjectList::Cleanup() {
   }
 }
 
-void BaseObjectList::MemoryInfo(node::MemoryTracker* tracker) const {
+void BaseObjectList::MemoryInfo(MemoryTracker* tracker) const {
   for (auto bo : *this) {
-    if (bo->IsDoneInitializing()) tracker->Track(bo);
+    if (bo->IsDoneInitializing()) {
+      // TODO(addaleax): Add weak edges instead of no edges once
+      // https://github.com/v8/v8/commit/e37cadf1143a8c5bbe44c0408186b5a26cc23863
+      // is available for us
+      tracker->Track(
+          bo, bo->persistent().IsWeak() ? MemoryTracker::kNoEdge : nullptr);
+    }
   }
 }
 
+const char* const MemoryTracker::kNoEdge = "<MemoryTracker::kNoEdge>";
+
 }  // namespace node

src/cppgc_helpers.cc

@@ -16,7 +16,10 @@ void CppgcWrapperList::MemoryInfo(MemoryTracker* tracker) const {
   for (auto node : *this) {
     CppgcMixin* ptr = node->persistent.Get();
     if (ptr != nullptr) {
-      tracker->Track(ptr);
+      // TODO(addaleax): Add weak edges instead of no edges once
+      // https://github.com/v8/v8/commit/e37cadf1143a8c5bbe44c0408186b5a26cc23863
+      // is available for us
+      tracker->Track(ptr, MemoryTracker::kNoEdge);
     }
   }
 }

src/memory_tracker-inl.h

@@ -131,7 +131,7 @@ void MemoryTracker::TrackField(const char* edge_name,
   if (value == nullptr) return;
   auto it = seen_.find(value);
   if (it != seen_.end()) {
-    graph_->AddEdge(CurrentNode(), it->second, edge_name);
+    AddEdge(CurrentNode(), it->second, edge_name);
   } else {
     Track(value, edge_name);
   }
@@ -253,9 +253,9 @@ void MemoryTracker::TrackField(const char* edge_name,
                                const v8::Local<T>& value,
                                const char* node_name) {
   if (!value.IsEmpty())
-    graph_->AddEdge(CurrentNode(),
-                    graph_->V8Node(value.template As<v8::Value>()),
-                    edge_name);
+    AddEdge(CurrentNode(),
+            graph_->V8Node(value.template As<v8::Value>()),
+            edge_name);
 }
 
 template <typename T>
@@ -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) {
-      graph_->AddEdge(CurrentNode(), it->second, edge_name);
+      AddEdge(CurrentNode(), it->second, edge_name);
     }
     return;  // It has already been tracked, no need to call MemoryInfo again
   }
@@ -317,7 +317,7 @@ void MemoryTracker::Track(const MemoryRetainer* retainer,
   auto it = seen_.find(retainer);
   if (it != seen_.end()) {
     if (CurrentNode() != nullptr) {
-      graph_->AddEdge(CurrentNode(), it->second, edge_name);
+      AddEdge(CurrentNode(), it->second, edge_name);
     }
     return;  // It has already been tracked, no need to call MemoryInfo again
   }
@@ -376,7 +376,7 @@ MemoryRetainerNode* MemoryTracker::AddNode(const CppgcMixin* retainer,
   MemoryRetainerNode* n = new MemoryRetainerNode(this, retainer);
   seen_[retainer] = n;
   if (CurrentNode() != nullptr) {
-    graph_->AddEdge(CurrentNode(), n->JSWrapperNode(), edge_name);
+    AddEdge(CurrentNode(), n->JSWrapperNode(), edge_name);
   }
 
   return n;
@@ -392,11 +392,11 @@ MemoryRetainerNode* MemoryTracker::AddNode(const MemoryRetainer* retainer,
   MemoryRetainerNode* n = new MemoryRetainerNode(this, retainer);
   graph_->AddNode(std::unique_ptr<v8::EmbedderGraph::Node>(n));
   seen_[retainer] = n;
-  if (CurrentNode() != nullptr) graph_->AddEdge(CurrentNode(), n, edge_name);
+  if (CurrentNode() != nullptr) AddEdge(CurrentNode(), n, edge_name);
 
   if (n->JSWrapperNode() != nullptr) {
-    graph_->AddEdge(n, n->JSWrapperNode(), "native_to_javascript");
-    graph_->AddEdge(n->JSWrapperNode(), n, "javascript_to_native");
+    AddEdge(n, n->JSWrapperNode(), "native_to_javascript");
+    AddEdge(n->JSWrapperNode(), n, "javascript_to_native");
   }
 
   return n;
@@ -408,7 +408,7 @@ MemoryRetainerNode* MemoryTracker::AddNode(const char* node_name,
   MemoryRetainerNode* n = new MemoryRetainerNode(this, node_name, size);
   graph_->AddNode(std::unique_ptr<v8::EmbedderGraph::Node>(n));
 
-  if (CurrentNode() != nullptr) graph_->AddEdge(CurrentNode(), n, edge_name);
+  if (CurrentNode() != nullptr) AddEdge(CurrentNode(), n, edge_name);
 
   return n;
 }
@@ -435,6 +435,13 @@ MemoryRetainerNode* MemoryTracker::PushNode(const char* node_name,
   return n;
 }
 
+void MemoryTracker::AddEdge(v8::EmbedderGraph::Node* from,
+                            v8::EmbedderGraph::Node* to,
+                            const char* edge_name) {
+  if (edge_name == kNoEdge) return;
+  graph_->AddEdge(from, to, edge_name);
+}
+
 void MemoryTracker::PopNode() {
   node_stack_.pop();
 }

src/memory_tracker.h

@@ -300,6 +300,11 @@ class MemoryTracker {
                                 v8::EmbedderGraph* graph)
     : isolate_(isolate), graph_(graph) {}
 
+  // Can be passed to Track() if it is _not_ desirable
+  // to create an edge between nodes, and the target node is
+  // only supposed to be added to the graph, not (yet) connected.
+  static const char* const kNoEdge;
+
  private:
   typedef std::unordered_map<const MemoryRetainer*, MemoryRetainerNode*>
       NodeMap;
@@ -321,6 +326,9 @@ class MemoryTracker {
                                       size_t size,
                                       const char* edge_name = nullptr);
   inline void PopNode();
+  inline void AddEdge(v8::EmbedderGraph::Node* from,
+                      v8::EmbedderGraph::Node* to,
+                      const char* edge_name = nullptr);
 
   v8::Isolate* isolate_;
   v8::EmbedderGraph* graph_;

test/common/heap.js

@@ -336,11 +336,25 @@ function validateByRetainingPath(...args) {
   return validateByRetainingPathFromNodes(nodes, ...args);
 }
 
+function getRetainingNodes(startingNode, filter) {
+  const seen = new Set();
+  function listNodes(node) {
+    if (!filter(node) || seen.has(node)) return;
+    seen.add(node);
+    for (const edge of node.incomingEdges) {
+      listNodes(edge.from);
+    }
+  }
+  listNodes(startingNode);
+  return [...seen];
+}
+
 module.exports = {
   recordState,
   validateSnapshotNodes,
   validateByRetainingPath,
   validateByRetainingPathFromNodes,
   getHeapSnapshotOptionTests,
   createJSHeapSnapshot,
+  getRetainingNodes,
 };

test/pummel/test-heapdump-zlib.js

@@ -3,7 +3,7 @@
 
 const common = require('../common');
 const assert = require('assert');
-const { validateByRetainingPath, validateByRetainingPathFromNodes } = require('../common/heap');
+const { validateByRetainingPath, validateByRetainingPathFromNodes, getRetainingNodes } = require('../common/heap');
 const zlib = require('zlib');
 
 // Before zlib stream is created, no ZlibStream should be created.
@@ -27,6 +27,18 @@ const gzip = zlib.createGzip();
   assert.strictEqual(withMemory.length, 0);
 }
 
+{
+  // Assert that the `ZlibStream` has no unexpected connections
+  // to other `Node / ...` nodes.
+  const contexts = validateByRetainingPath('Node / ZlibContext', []);
+  assert.deepStrictEqual(
+    getRetainingNodes(contexts[0], (node) => node.name?.startsWith('Node /'))
+      .map((node) => node.name).sort(), [
+      'Node / ZlibContext',
+      'Node / ZlibStream',
+    ]);
+}
+
 // After zlib stream is written, zlib_memory should be created.
 gzip.write('hello world', common.mustCall(() => {
   validateByRetainingPath('Node / ZlibStream', [