From daf3152f7e933672f04663d4f7e1d89b59062bf7 Mon Sep 17 00:00:00 2001
From: Joyee Cheung <joyeec9h3@gmail.com>
Date: Tue, 27 Sep 2022 14:15:51 +0800
Subject: [PATCH] src: implement GetDetachedness() in MemoryRetainerNode

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: https://github.com/nodejs/node/pull/44803
Reviewed-By: Chengzhong Wu <legendecas@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
---
 src/base_object-inl.h            |  5 +++++
 src/base_object.h                |  2 ++
 src/memory_tracker-inl.h         |  6 ++++++
 src/memory_tracker.h             |  3 +++
 test/common/heap.js              | 16 ++++++++++++++++
 test/pummel/test-heapdump-dns.js |  3 ++-
 6 files changed, 34 insertions(+), 1 deletion(-)

diff --git a/src/base_object-inl.h b/src/base_object-inl.h
index a246c2502b48e1..e27de6ef32e8b3 100644
--- a/src/base_object-inl.h
+++ b/src/base_object-inl.h
@@ -92,6 +92,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
index a17879be5b452a..8dc0a194185237 100644
--- a/src/base_object.h
+++ b/src/base_object.h
@@ -85,6 +85,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
index 0ba44f1f16efbb..4757bee734d738 100644
--- 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
index 1896624bbeb9dd..878938bc48532e 100644
--- 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
index 2d1ea2c8179cd1..6e5e55b000341b 100644
--- 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
index 675ddc09aca61a..faadcb6a642900 100644
--- 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
   },
 ]);