src: track ShadowRealm native objects correctly in the heap snapshot

Previously the native ShadowRealm objects were never actually tracked in the heap snapshot - the tracking starts with the Environment, we don't call the tracking methods unless it's reachable natively from the Environment. This patch adds a set in the Environment for tracking shadow realms in the heap snapshot. Drive-by: remove redundant MemoryInfo() overrides from the derived classes of Realm and remove the tracking of `env` from `Realm::MemoryInfo()` because the realms don't hold the Environment alive (even for the principal realm, it's the other way around). PR-URL: #47389 Reviewed-By: Chengzhong Wu <[email protected]> Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: Colin Ihrig <[email protected]>
nodejs · Apr 13, 2023 · 5250947 · 5250947
1 parent 9a4d21d
commit 5250947
Show file tree

Hide file tree

Showing 6 changed files with 30 additions and 9 deletions.
diff --git a/src/env.cc b/src/env.cc
@@ -11,6 +11,7 @@
 #include "node_internals.h"
 #include "node_options-inl.h"
 #include "node_process-inl.h"
+#include "node_shadow_realm.h"
 #include "node_v8_platform-inl.h"
 #include "node_worker.h"
 #include "req_wrap-inl.h"
@@ -226,6 +227,14 @@ void Environment::UntrackContext(Local<Context> context) {
   }
 }
 
+void Environment::TrackShadowRealm(shadow_realm::ShadowRealm* realm) {
+  shadow_realms_.insert(realm);
+}
+
+void Environment::UntrackShadowRealm(shadow_realm::ShadowRealm* realm) {
+  shadow_realms_.erase(realm);
+}
+
 AsyncHooks::DefaultTriggerAsyncIdScope::DefaultTriggerAsyncIdScope(
     Environment* env, double default_trigger_async_id)
     : async_hooks_(env->async_hooks()) {
@@ -901,6 +910,10 @@ Environment::~Environment() {
       addon.Close();
     }
   }
+
+  for (auto realm : shadow_realms_) {
+    realm->OnEnvironmentDestruct();
+  }
 }
 
 void Environment::InitializeLibuv() {
@@ -1896,6 +1909,7 @@ void Environment::MemoryInfo(MemoryTracker* tracker) const {
   tracker->TrackField("timeout_info", timeout_info_);
   tracker->TrackField("tick_info", tick_info_);
   tracker->TrackField("principal_realm", principal_realm_);
+  tracker->TrackField("shadow_realms", shadow_realms_);
 
   // FIXME(joyeecheung): track other fields in Environment.
   // Currently MemoryTracker is unable to track these

diff --git a/src/env.h b/src/env.h
@@ -64,6 +64,9 @@
 
 namespace node {
 
+namespace shadow_realm {
+class ShadowRealm;
+}
 namespace contextify {
 class ContextifyScript;
 class CompiledFnEntry;
@@ -643,6 +646,8 @@ class Environment : public MemoryRetainer {
                        const ContextInfo& info);
   void TrackContext(v8::Local<v8::Context> context);
   void UntrackContext(v8::Local<v8::Context> context);
+  void TrackShadowRealm(shadow_realm::ShadowRealm* realm);
+  void UntrackShadowRealm(shadow_realm::ShadowRealm* realm);
 
   void StartProfilerIdleNotifier();
 
@@ -1020,6 +1025,7 @@ class Environment : public MemoryRetainer {
 
   size_t async_callback_scope_depth_ = 0;
   std::vector<double> destroy_async_id_list_;
+  std::unordered_set<shadow_realm::ShadowRealm*> shadow_realms_;
 
 #if HAVE_INSPECTOR
   std::unique_ptr<profiler::V8CoverageConnection> coverage_connection_;

diff --git a/src/node_realm.cc b/src/node_realm.cc
@@ -33,7 +33,6 @@ void Realm::MemoryInfo(MemoryTracker* tracker) const {
   PER_REALM_STRONG_PERSISTENT_VALUES(V)
 #undef V
 
-  tracker->TrackField("env", env_);
   tracker->TrackField("cleanup_queue", cleanup_queue_);
   tracker->TrackField("builtins_with_cache", builtins_with_cache);
   tracker->TrackField("builtins_without_cache", builtins_without_cache);
@@ -301,10 +300,6 @@ PrincipalRealm::PrincipalRealm(Environment* env,
   }
 }
 
-void PrincipalRealm::MemoryInfo(MemoryTracker* tracker) const {
-  Realm::MemoryInfo(tracker);
-}
-
 MaybeLocal<Value> PrincipalRealm::BootstrapRealm() {
   HandleScope scope(isolate_);
 

diff --git a/src/node_realm.h b/src/node_realm.h
@@ -165,7 +165,6 @@ class PrincipalRealm : public Realm {
 
   SET_MEMORY_INFO_NAME(PrincipalRealm)
   SET_SELF_SIZE(PrincipalRealm)
-  void MemoryInfo(MemoryTracker* tracker) const override;
 
 #define V(PropertyName, TypeName)                                              \
   v8::Local<TypeName> PropertyName() const override;                           \

diff --git a/src/node_shadow_realm.cc b/src/node_shadow_realm.cc
@@ -46,6 +46,7 @@ void ShadowRealm::WeakCallback(const v8::WeakCallbackInfo<ShadowRealm>& data) {
 
 ShadowRealm::ShadowRealm(Environment* env)
     : Realm(env, NewContext(env->isolate()), kShadowRealm) {
+  env->TrackShadowRealm(this);
   context_.SetWeak(this, WeakCallback, v8::WeakCallbackType::kParameter);
   CreateProperties();
 }
@@ -54,10 +55,15 @@ ShadowRealm::~ShadowRealm() {
   while (HasCleanupHooks()) {
     RunCleanup();
   }
+  if (env_ != nullptr) {
+    env_->UntrackShadowRealm(this);
+  }
 }
 
-void ShadowRealm::MemoryInfo(MemoryTracker* tracker) const {
-  Realm::MemoryInfo(tracker);
+void ShadowRealm::OnEnvironmentDestruct() {
+  CHECK_NOT_NULL(env_);
+  env_ = nullptr;  // This means that the shadow realm has out-lived the
+                   // environment.
 }
 
 v8::Local<v8::Context> ShadowRealm::context() const {

diff --git a/src/node_shadow_realm.h b/src/node_shadow_realm.h
@@ -15,7 +15,6 @@ class ShadowRealm : public Realm {
 
   SET_MEMORY_INFO_NAME(ShadowRealm)
   SET_SELF_SIZE(ShadowRealm)
-  void MemoryInfo(MemoryTracker* tracker) const override;
 
   v8::Local<v8::Context> context() const override;
 
@@ -25,6 +24,8 @@ class ShadowRealm : public Realm {
   PER_REALM_STRONG_PERSISTENT_VALUES(V)
 #undef V
 
+  void OnEnvironmentDestruct();
+
  protected:
   v8::MaybeLocal<v8::Value> BootstrapRealm() override;