src: implement GetDetachedness() in MemoryRetainerNode

joyeecheung · joyeecheung · commit 3ab2d4f362b4 · 2022-10-05T19:38:39.000+08:00
This allows us to mark weak/detached references in the heap snapshot. Also mark weak/detached BaseObject with Detachedness::kDetached so that the state of the reference can be displayed by frontend consuming the heap snapshot. PR-URL: nodejs#44803 Reviewed-By: Chengzhong Wu <legendecas@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com>
diff --git a/src/base_object-inl.h b/src/base_object-inl.h
@@ -115,6 +115,11 @@ 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(
     v8::Local<v8::String> property,
diff --git a/src/base_object.h b/src/base_object.h
@@ -96,6 +96,8 @@ 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`.
diff --git a/src/memory_tracker-inl.h b/src/memory_tracker-inl.h
@@ -31,6 +31,7 @@ class MemoryRetainerNode : public v8::EmbedderGraph::Node {
 
     name_ = retainer_->MemoryInfoName();
     size_ = retainer_->SelfSize();
+    detachedness_ = retainer_->GetDetachedness();
   }
 
   inline MemoryRetainerNode(MemoryTracker* tracker,
@@ -57,6 +58,9 @@ class MemoryRetainerNode : public v8::EmbedderGraph::Node {
     }
     return is_root_node_;
   }
+  v8::EmbedderGraph::Node::Detachedness GetDetachedness() override {
+    return detachedness_;
+  }
 
  private:
   friend class MemoryTracker;
@@ -73,6 +77,8 @@ class MemoryRetainerNode : public v8::EmbedderGraph::Node {
   bool is_root_node_ = false;
   std::string name_;
   size_t size_ = 0;
+  v8::EmbedderGraph::Node::Detachedness detachedness_ =
+      v8::EmbedderGraph::Node::Detachedness::kUnknown;
 };
 
 void MemoryTracker::TrackFieldWithSize(const char* edge_name,
diff --git a/src/memory_tracker.h b/src/memory_tracker.h
@@ -127,6 +127,9 @@ class MemoryRetainer {
   }
 
   virtual bool IsRootNode() const { return false; }
+  virtual v8::EmbedderGraph::Node::Detachedness GetDetachedness() const {
+    return v8::EmbedderGraph::Node::Detachedness::kUnknown;
+  }
 };
 
 class MemoryTracker {
diff --git a/test/common/heap.js b/test/common/heap.js
@@ -142,6 +142,22 @@ class State {
           }
         }
       }
+
+      if (expectation.detachedness !== undefined) {
+        const matchedNodes = rootNodes.filter(
+          (node) => node.detachedness === expectation.detachedness);
+        if (loose) {
+          assert(matchedNodes.length >= rootNodes.length,
+                 `Expect to find at least ${rootNodes.length} with ` +
+                `detachedness ${expectation.detachedness}, ` +
+                `found ${matchedNodes.length}`);
+        } else {
+          assert.strictEqual(
+            matchedNodes.length, rootNodes.length,
+            `Expect to find ${rootNodes.length} with detachedness ` +
+            `${expectation.detachedness},  found ${matchedNodes.length}`);
+        }
+      }
     }
   }
 
diff --git a/test/pummel/test-heapdump-dns.js b/test/pummel/test-heapdump-dns.js
@@ -13,6 +13,7 @@ validateSnapshotNodes('Node / ChannelWrap', [
       { node_name: 'Node / NodeAresTask::List', edge_name: 'task_list' },
       // `Node / ChannelWrap` (C++) -> `ChannelWrap` (JS)
       { node_name: 'ChannelWrap', edge_name: 'wrapped' },
-    ]
+    ],
+    detachedness: 2
   },
 ]);

Original file line number	Diff line number	Diff line change
`@@ -127,6 +127,9 @@ class MemoryRetainer {`
`127`	`127`	`}`
`128`	`128`
`129`	`129`	`virtual bool IsRootNode() const { return false; }`
	`130`	`+ virtual v8::EmbedderGraph::Node::Detachedness GetDetachedness() const {`
	`131`	`+ return v8::EmbedderGraph::Node::Detachedness::kUnknown;`
	`132`	`+ }`
`130`	`133`	`};`
`131`	`134`
`132`	`135`	`class MemoryTracker {`
Original file line number	Diff line number	Diff line change
`@@ -142,6 +142,22 @@ class State {`
`142`	`142`	`}`
`143`	`143`	`}`
`144`	`144`	`}`
	`145`	`+`
	`146`	`+ if (expectation.detachedness !== undefined) {`
	`147`	`+ const matchedNodes = rootNodes.filter(`
	`148`	`+ (node) => node.detachedness === expectation.detachedness);`
	`149`	`+ if (loose) {`
	`150`	`+ assert(matchedNodes.length >= rootNodes.length,`
	`151`	+ `Expect to find at least ${rootNodes.length} with ` +
	`152`	+ `detachedness ${expectation.detachedness}, ` +
	`153`	+ `found ${matchedNodes.length}`);
	`154`	`+ } else {`
	`155`	`+ assert.strictEqual(`
	`156`	`+ matchedNodes.length, rootNodes.length,`
	`157`	+ `Expect to find ${rootNodes.length} with detachedness ` +
	`158`	+ `${expectation.detachedness}, found ${matchedNodes.length}`);
	`159`	`+ }`
	`160`	`+ }`
`145`	`161`	`}`
`146`	`162`	`}`
`147`	`163`