src: do not track weak BaseObjects as childrens of Realms

addaleax · nodejs-github-bot · commit 2fb168f75648 · 2026-06-13T23:12:35.000Z
Heap dumps are used for analyzing the relationship between objects that keep each other alive, so that retainers of memory can be detected and memory leaks fixed. 6e60ab7 broke this for a wide range of `BaseObject` instances. Marking all `BaseObject`s, including weak ones, as children of `Realm` and `Environment` instances gives the incorrect impression that they are held alive by those objects, effectively hiding the "real" set of strong roots that keep other objects alive through GC. An upcoming change in V8 will allow us to represent this relationship between `Realm` and `Environment` and its `BaseObject` and `CppgcMixin` instances properly. Refs: v8/v8@e37cadf Refs: #57417 Signed-off-by: Anna Henningsen <anna@addaleax.net> PR-URL: #63842 Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com> Reviewed-By: Chengzhong Wu <legendecas@gmail.com>
diff --git a/src/base_object.cc b/src/base_object.cc
@@ -174,9 +174,15 @@ 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::kWeakEdge : nullptr);
+    }
   }
 }
 
diff --git a/src/cppgc_helpers.cc b/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::kWeakEdge);
     }
   }
 }
diff --git a/test/common/heap.js b/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,
 };
diff --git a/test/pummel/test-heapdump-zlib.js b/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', [

Original file line number	Diff line number	Diff line change
`@@ -174,9 +174,15 @@ void BaseObjectList::Cleanup() {`
`174`	`174`	`}`
`175`	`175`	`}`
`176`	`176`
`177`		`-void BaseObjectList::MemoryInfo(node::MemoryTracker* tracker) const {`
	`177`	`+void BaseObjectList::MemoryInfo(MemoryTracker* tracker) const {`
`178`	`178`	`for (auto bo : *this) {`
`179`		`- if (bo->IsDoneInitializing()) tracker->Track(bo);`
	`179`	`+ if (bo->IsDoneInitializing()) {`
	`180`	`+ // TODO(addaleax): Add weak edges instead of no edges once`
	`181`	`+ // https://github.com/v8/v8/commit/e37cadf1143a8c5bbe44c0408186b5a26cc23863`
	`182`	`+ // is available for us`
	`183`	`+ tracker->Track(`
	`184`	`+ bo, bo->persistent().IsWeak() ? MemoryTracker::kWeakEdge : nullptr);`
	`185`	`+ }`
`180`	`186`	`}`
`181`	`187`	`}`
`182`	`188`
Original file line number	Diff line number	Diff line change
`@@ -16,7 +16,10 @@ void CppgcWrapperList::MemoryInfo(MemoryTracker* tracker) const {`
`16`	`16`	`for (auto node : *this) {`
`17`	`17`	`CppgcMixin* ptr = node->persistent.Get();`
`18`	`18`	`if (ptr != nullptr) {`
`19`		`- tracker->Track(ptr);`
	`19`	`+ // TODO(addaleax): Add weak edges instead of no edges once`
	`20`	`+ // https://github.com/v8/v8/commit/e37cadf1143a8c5bbe44c0408186b5a26cc23863`
	`21`	`+ // is available for us`
	`22`	`+ tracker->Track(ptr, MemoryTracker::kWeakEdge);`
`20`	`23`	`}`
`21`	`24`	`}`
`22`	`25`	`}`