src: annotate BaseObjects in the heap snapshots correctly

src/base_object-inl.h

@@ -125,11 +125,6 @@ bool BaseObject::IsWeakOrDetached() const {
   return pd->wants_weak_jsobj || pd->is_detached;
 }
 
-v8::EmbedderGraph::Node::Detachedness BaseObject::GetDetachedness() const {
-  return IsWeakOrDetached() ? v8::EmbedderGraph::Node::Detachedness::kDetached
-                            : v8::EmbedderGraph::Node::Detachedness::kUnknown;
-}
-
 template <int Field>
 void BaseObject::InternalFieldGet(
     const v8::FunctionCallbackInfo<v8::Value>& args) {

src/base_object.cc

@@ -161,10 +161,6 @@ Local<Object> BaseObject::WrappedObject() const {
   return object();
 }
 
-bool BaseObject::IsRootNode() const {
-  return !persistent_handle_.IsWeak();
-}
-
 bool BaseObject::IsNotIndicativeOfMemoryLeakAtExit() const {
   return IsWeakOrDetached();
 }

src/base_object.h

@@ -105,8 +105,6 @@ class BaseObject : public MemoryRetainer {
   // to it anymore.
   inline bool IsWeakOrDetached() const;
 
-  inline v8::EmbedderGraph::Node::Detachedness GetDetachedness() const override;
-
   // Utility to create a FunctionTemplate with one internal field (used for
   // the `BaseObject*` pointer) and a constructor that initializes that field
   // to `nullptr`.
@@ -192,7 +190,6 @@ class BaseObject : public MemoryRetainer {
 
  private:
   v8::Local<v8::Object> WrappedObject() const override;
-  bool IsRootNode() const override;
   void DeleteMe();
 
   // persistent_handle_ needs to be at a fixed offset from the start of the

test/common/heap.js

@@ -221,6 +221,7 @@ function validateSnapshotNodes(...args) {
  * A alternative heap snapshot validator that can be used to verify cppgc-managed nodes.
  * Modified from
  * https://chromium.googlesource.com/v8/v8/+/b00e995fb212737802810384ba2b868d0d92f7e5/test/unittests/heap/cppgc-js/unified-heap-snapshot-unittest.cc#134
+ * @param {object[]} nodes Snapshot nodes returned by createJSHeapSnapshot() or a subset filtered from it.
  * @param {string} rootName Name of the root node. Typically a class name used to filter all native nodes with
  *                          this name. For cppgc-managed objects, this is typically the name configured by
  *                          SET_CPPGC_NAME() prefixed with an additional "Node /" prefix e.g.
@@ -231,12 +232,12 @@ function validateSnapshotNodes(...args) {
  *   node_type?: string,
  *   edge_type?: string,
  * }]} retainingPath The retaining path specification to search from the root nodes.
+ * @param {boolean} allowEmpty Whether the function should fail if no matching nodes can be found.
  * @returns {[object]} All the leaf nodes matching the retaining path specification. If none can be found,
  *                     logs the nodes found in the last matching step of the path (if any), and throws an
  *                     assertion error.
  */
-function findByRetainingPath(rootName, retainingPath) {
-  const nodes = createJSHeapSnapshot();
+function validateByRetainingPathFromNodes(nodes, rootName, retainingPath, allowEmpty = false) {
   let haystack = nodes.filter((n) => n.name === rootName && n.type !== 'string');
 
   for (let i = 0; i < retainingPath.length; ++i) {
@@ -269,6 +270,9 @@ function findByRetainingPath(rootName, retainingPath) {
     }
 
     if (newHaystack.length === 0) {
+      if (allowEmpty) {
+        return [];
+      }
       const format = (val) => util.inspect(val, { breakLength: 128, depth: 3 });
       console.error('#');
       console.error('# Retaining path to search for:');
@@ -282,6 +286,7 @@ function findByRetainingPath(rootName, retainingPath) {
       }
 
       assert.fail(`Could not find target edge ${format(expected)} in the heap snapshot.`);
+
     }
 
     haystack = newHaystack;
@@ -321,9 +326,19 @@ function getHeapSnapshotOptionTests() {
   };
 }
 
+/**
+ * Similar to @see {validateByRetainingPathFromNodes} but creates the snapshot from scratch.
+ */
+function validateByRetainingPath(...args) {
+  const nodes = createJSHeapSnapshot();
+  return validateByRetainingPathFromNodes(nodes, ...args);
+}
+
 module.exports = {
   recordState,
   validateSnapshotNodes,
-  findByRetainingPath,
+  validateByRetainingPath,
+  validateByRetainingPathFromNodes,
   getHeapSnapshotOptionTests,
+  createJSHeapSnapshot,
 };

test/pummel/test-heapdump-dns.js

@@ -1,19 +1,31 @@
-// Flags: --expose-internals
 'use strict';
+// This tests heap snapshot integration of dns ChannelWrap.
+
 require('../common');
-const { validateSnapshotNodes } = require('../common/heap');
+const { validateByRetainingPath } = require('../common/heap');
+const assert = require('assert');
+
+// Before dns is loaded, no ChannelWrap should be created.
+{
+  const nodes = validateByRetainingPath('Node / ChannelWrap', []);
+  assert.strictEqual(nodes.length, 0);
+}
 
-validateSnapshotNodes('Node / ChannelWrap', []);
 const dns = require('dns');
-validateSnapshotNodes('Node / ChannelWrap', [{}]);
+
+// Right after dns is loaded, a ChannelWrap should be created for the default
+// resolver, but it has no task list.
+{
+  validateByRetainingPath('Node / ChannelWrap', [
+    { node_name: 'ChannelWrap', edge_name: 'native_to_javascript' },
+  ]);
+}
+
 dns.resolve('localhost', () => {});
-validateSnapshotNodes('Node / ChannelWrap', [
-  {
-    children: [
-      { node_name: 'Node / NodeAresTask::List', edge_name: 'task_list' },
-      // `Node / ChannelWrap` (C++) -> `ChannelWrap` (JS)
-      { node_name: 'ChannelWrap', edge_name: 'native_to_javascript' },
-    ],
-    detachedness: 2,
-  },
-]);
+
+// After dns resolution, the ChannelWrap of the default resolver has the task list.
+{
+  validateByRetainingPath('Node / ChannelWrap', [
+    { node_name: 'Node / NodeAresTask::List', edge_name: 'task_list' },
+  ]);
+}

test/pummel/test-heapdump-env.js

@@ -1,24 +1,28 @@
-// Flags: --expose-internals
 'use strict';
 
 // This tests that Environment is tracked in heap snapshots.
 // Tests for BaseObject and cppgc-managed objects are done in other
 // test-heapdump-*.js files.
 
 require('../common');
-const { validateSnapshotNodes } = require('../common/heap');
+const { createJSHeapSnapshot, validateByRetainingPathFromNodes } = require('../common/heap');
 
-validateSnapshotNodes('Node / Environment', [{
-  children: [
-    { node_name: 'Node / CleanupQueue', edge_name: 'cleanup_queue' },
-    { node_name: 'Node / IsolateData', edge_name: 'isolate_data' },
-    { node_name: 'Node / PrincipalRealm', edge_name: 'principal_realm' },
-  ],
-}]);
+const nodes = createJSHeapSnapshot();
 
-validateSnapshotNodes('Node / PrincipalRealm', [{
-  children: [
-    { node_name: 'process', edge_name: 'process_object' },
-    { node_name: 'Node / BaseObjectList', edge_name: 'base_object_list' },
-  ],
-}]);
+const envs = validateByRetainingPathFromNodes(nodes, 'Node / Environment', []);
+validateByRetainingPathFromNodes(envs, 'Node / Environment', [
+  { node_name: 'Node / CleanupQueue', edge_name: 'cleanup_queue' },
+]);
+validateByRetainingPathFromNodes(envs, 'Node / Environment', [
+  { node_name: 'Node / IsolateData', edge_name: 'isolate_data' },
+]);
+
+const realms = validateByRetainingPathFromNodes(envs, 'Node / Environment', [
+  { node_name: 'Node / PrincipalRealm', edge_name: 'principal_realm' },
+]);
+validateByRetainingPathFromNodes(realms, 'Node / PrincipalRealm', [
+  { node_name: 'process', edge_name: 'process_object' },
+]);
+validateByRetainingPathFromNodes(realms, 'Node / PrincipalRealm', [
+  { node_name: 'Node / BaseObjectList', edge_name: 'base_object_list' },
+]);

test/pummel/test-heapdump-fs-promise.js

@@ -1,16 +1,26 @@
-// Flags: --expose-internals
 'use strict';
+
+// This tests heap snapshot integration of fs promise.
+
 require('../common');
-const { validateSnapshotNodes } = require('../common/heap');
+const { validateByRetainingPath, validateByRetainingPathFromNodes } = require('../common/heap');
 const fs = require('fs').promises;
+const assert = require('assert');
+
+// Before fs promise is used, no FSReqPromise should be created.
+{
+  const nodes = validateByRetainingPath('Node / FSReqPromise', []);
+  assert.strictEqual(nodes.length, 0);
+}
 
-validateSnapshotNodes('Node / FSReqPromise', []);
 fs.stat(__filename);
-validateSnapshotNodes('Node / FSReqPromise', [
-  {
-    children: [
-      { node_name: 'FSReqPromise', edge_name: 'native_to_javascript' },
-      { node_name: 'Node / AliasedFloat64Array', edge_name: 'stats_field_array' },
-    ],
-  },
-]);
+
+{
+  const nodes = validateByRetainingPath('Node / FSReqPromise', []);
+  validateByRetainingPathFromNodes(nodes, 'Node / FSReqPromise', [
+    { node_name: 'FSReqPromise', edge_name: 'native_to_javascript' },
+  ]);
+  validateByRetainingPathFromNodes(nodes, 'Node / FSReqPromise', [
+    { node_name: 'Node / AliasedFloat64Array', edge_name: 'stats_field_array' },
+  ]);
+}

test/pummel/test-heapdump-http2.js

@@ -1,15 +1,21 @@
-// Flags: --expose-internals
 'use strict';
+
+// This tests heap snapshot integration of http2.
+
 const common = require('../common');
-const { recordState } = require('../common/heap');
+const { createJSHeapSnapshot, validateByRetainingPath, validateByRetainingPathFromNodes } = require('../common/heap');
+const assert = require('assert');
+
 if (!common.hasCrypto)
   common.skip('missing crypto');
 const http2 = require('http2');
 
+// Before http2 is used, no Http2Session or Http2Streamshould be created.
 {
-  const state = recordState();
-  state.validateSnapshotNodes('Node / Http2Session', []);
-  state.validateSnapshotNodes('Node / Http2Stream', []);
+  const sessions = validateByRetainingPath('Node / Http2Session', []);
+  assert.strictEqual(sessions.length, 0);
+  const streams = validateByRetainingPath('Node / Http2Stream', []);
+  assert.strictEqual(streams.length, 0);
 }
 
 const server = http2.createServer();
@@ -21,63 +27,45 @@ server.listen(0, () => {
   const req = client.request();
 
   req.on('response', common.mustCall(() => {
-    const state = recordState();
+    const nodes = createJSHeapSnapshot();
 
     // `Node / Http2Stream` (C++) -> Http2Stream (JS)
-    state.validateSnapshotNodes('Node / Http2Stream', [
-      {
-        children: [
-          // current_headers and/or queue could be empty
-          { node_name: 'Http2Stream', edge_name: 'native_to_javascript' },
-        ],
-      },
-    ], { loose: true });
+    validateByRetainingPathFromNodes(nodes, 'Node / Http2Stream', [
+      // current_headers and/or queue could be empty
+      { node_name: 'Http2Stream', edge_name: 'native_to_javascript' },
+    ]);
 
     // `Node / FileHandle` (C++) -> FileHandle (JS)
-    state.validateSnapshotNodes('Node / FileHandle', [
-      {
-        children: [
-          { node_name: 'FileHandle', edge_name: 'native_to_javascript' },
-          // current_headers could be empty
-        ],
-      },
-    ], { loose: true });
-    state.validateSnapshotNodes('Node / TCPSocketWrap', [
-      {
-        children: [
-          { node_name: 'TCP', edge_name: 'native_to_javascript' },
-        ],
-      },
-    ], { loose: true });
-    state.validateSnapshotNodes('Node / TCPServerWrap', [
-      {
-        children: [
-          { node_name: 'TCP', edge_name: 'native_to_javascript' },
-        ],
-      },
-    ], { loose: true });
+    validateByRetainingPathFromNodes(nodes, 'Node / FileHandle', [
+      { node_name: 'FileHandle', edge_name: 'native_to_javascript' },
+      // current_headers could be empty
+    ]);
+    validateByRetainingPathFromNodes(nodes, 'Node / TCPSocketWrap', [
+      { node_name: 'TCP', edge_name: 'native_to_javascript' },
+    ]);
+
+    validateByRetainingPathFromNodes(nodes, 'Node / TCPServerWrap', [
+      { node_name: 'TCP', edge_name: 'native_to_javascript' },
+    ]);
+
     // `Node / StreamPipe` (C++) -> StreamPipe (JS)
-    state.validateSnapshotNodes('Node / StreamPipe', [
-      {
-        children: [
-          { node_name: 'StreamPipe', edge_name: 'native_to_javascript' },
-        ],
-      },
+    validateByRetainingPathFromNodes(nodes, 'Node / StreamPipe', [
+      { node_name: 'StreamPipe', edge_name: 'native_to_javascript' },
     ]);
+
     // `Node / Http2Session` (C++) -> Http2Session (JS)
-    state.validateSnapshotNodes('Node / Http2Session', [
-      {
-        children: [
-          { node_name: 'Http2Session', edge_name: 'native_to_javascript' },
-          { node_name: 'Node / nghttp2_memory', edge_name: 'nghttp2_memory' },
-          {
-            node_name: 'Node / streams', edge_name: 'streams',
-          },
-          // outstanding_pings, outgoing_buffers, outgoing_storage,
-          // pending_rst_streams could be empty
-        ],
-      },
-    ], { loose: true });
+    const sessions = validateByRetainingPathFromNodes(nodes, 'Node / Http2Session', []);
+    validateByRetainingPathFromNodes(sessions, 'Node / Http2Session', [
+      { node_name: 'Http2Session', edge_name: 'native_to_javascript' },
+    ]);
+    validateByRetainingPathFromNodes(sessions, 'Node / Http2Session', [
+      { node_name: 'Node / nghttp2_memory', edge_name: 'nghttp2_memory' },
+    ]);
+    validateByRetainingPathFromNodes(sessions, 'Node / Http2Session', [
+      { node_name: 'Node / streams', edge_name: 'streams' },
+    ]);
+    // outstanding_pings, outgoing_buffers, outgoing_storage,
+    // pending_rst_streams could be empty
   }));
 
   req.resume();