Skip to content

Commit 38ff388

Browse files
committed
shadow_realm: fix memory leak by removing strong reference tracking
Shadow Realm contexts were never being garbage collected due to the Environment keeping strong C++ pointer references in the shadow_realms_ unordered_set. This created a circular dependency: 1. Environment holds strong pointer to ShadowRealm in shadow_realms_ set 2. ShadowRealm cannot be deleted because Environment holds it 3. V8 context is weak, but depends on ShadowRealm C++ object lifecycle 4. Circular leak - realms accumulate ~1.285 MB each, never freed The fix removes the TrackShadowRealm/UntrackShadowRealm mechanism entirely. Shadow Realms are now managed solely via: - Weak V8 context handle (context_.SetWeak()) - Cleanup hooks (AddCleanupHook/RemoveCleanupHook) - Weak callback (ShadowRealm::WeakCallback) This allows V8's garbage collector to properly reclaim Shadow Realm contexts when they become unreachable from JavaScript, as the C++ ShadowRealm object is no longer kept alive by a strong reference from the Environment. The existing test (test/parallel/test-shadow-realm-gc.js) verifies that Shadow Realms can be garbage collected with --expose-gc. Fixes: #47353
1 parent bfc81ca commit 38ff388

File tree

3 files changed

+18
-15
lines changed

3 files changed

+18
-15
lines changed

src/env.cc

Lines changed: 6 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -259,13 +259,9 @@ void Environment::UntrackContext(Local<Context> context) {
259259
}
260260
}
261261

262-
void Environment::TrackShadowRealm(shadow_realm::ShadowRealm* realm) {
263-
shadow_realms_.insert(realm);
264-
}
265-
266-
void Environment::UntrackShadowRealm(shadow_realm::ShadowRealm* realm) {
267-
shadow_realms_.erase(realm);
268-
}
262+
// TrackShadowRealm/UntrackShadowRealm removed - they created strong references
263+
// that prevented Shadow Realms from being garbage collected.
264+
// Fixes: https://github.com/nodejs/node/issues/47353
269265

270266
AsyncHooks::DefaultTriggerAsyncIdScope::DefaultTriggerAsyncIdScope(
271267
Environment* env, double default_trigger_async_id)
@@ -1044,8 +1040,8 @@ Environment::~Environment() {
10441040
inspector_agent_.reset();
10451041
#endif
10461042

1047-
// Sub-realms should have been cleared with Environment's cleanup.
1048-
DCHECK_EQ(shadow_realms_.size(), 0);
1043+
// Shadow realms are now managed via weak references and cleanup hooks,
1044+
// not tracked in a set. Fixes: https://github.com/nodejs/node/issues/47353
10491045
principal_realm_.reset();
10501046

10511047
if (trace_state_observer_) {
@@ -2232,7 +2228,7 @@ void Environment::MemoryInfo(MemoryTracker* tracker) const {
22322228
tracker->TrackField("timeout_info", timeout_info_);
22332229
tracker->TrackField("tick_info", tick_info_);
22342230
tracker->TrackField("principal_realm", principal_realm_);
2235-
tracker->TrackField("shadow_realms", shadow_realms_);
2231+
// shadow_realms_ removed - no longer tracking to avoid strong references
22362232

22372233
// FIXME(joyeecheung): track other fields in Environment.
22382234
// Currently MemoryTracker is unable to track these

src/env.h

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -691,8 +691,9 @@ class Environment final : public MemoryRetainer {
691691
Realm* realm,
692692
const ContextInfo& info);
693693
void UnassignFromContext(v8::Local<v8::Context> context);
694-
void TrackShadowRealm(shadow_realm::ShadowRealm* realm);
695-
void UntrackShadowRealm(shadow_realm::ShadowRealm* realm);
694+
// TrackShadowRealm/UntrackShadowRealm removed - they created strong
695+
// references that prevented Shadow Realms from being garbage collected.
696+
// Fixes: https://github.com/nodejs/node/issues/47353
696697

697698
void StartProfilerIdleNotifier();
698699

@@ -1117,7 +1118,9 @@ class Environment final : public MemoryRetainer {
11171118

11181119
size_t async_callback_scope_depth_ = 0;
11191120
std::vector<double> destroy_async_id_list_;
1120-
std::unordered_set<shadow_realm::ShadowRealm*> shadow_realms_;
1121+
// shadow_realms_ member removed - tracking shadow realms in a set created
1122+
// strong C++ references that prevented garbage collection.
1123+
// Fixes: https://github.com/nodejs/node/issues/47353
11211124

11221125
#if HAVE_INSPECTOR
11231126
std::unique_ptr<profiler::V8CoverageConnection> coverage_connection_;

src/node_shadow_realm.cc

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -79,7 +79,10 @@ ShadowRealm::ShadowRealm(Environment* env)
7979
context_.SetWeak(this, WeakCallback, v8::WeakCallbackType::kParameter);
8080
CreateProperties();
8181

82-
env->TrackShadowRealm(this);
82+
// Don't track shadow realms in environment's set - that creates a strong
83+
// reference preventing GC. The cleanup hook and weak callback are sufficient
84+
// for proper lifecycle management.
85+
// Fixes: https://github.com/nodejs/node/issues/47353
8386
env->AddCleanupHook(DeleteMe, this);
8487
}
8588

@@ -88,7 +91,8 @@ ShadowRealm::~ShadowRealm() {
8891
RunCleanup();
8992
}
9093

91-
env_->UntrackShadowRealm(this);
94+
// Removed UntrackShadowRealm() call - we no longer track shadow realms
95+
// in environment's set to avoid creating strong references that prevent GC.
9296

9397
if (context_.IsEmpty()) {
9498
// This most likely happened because the weak callback cleared it.

0 commit comments

Comments
 (0)