Skip to content

Commit 2af4064

Browse files
kayossouzaclaude
andcommitted
src: fix shadow realm memory leak by removing strong reference tracking
🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
1 parent bfc81ca commit 2af4064

File tree

4 files changed

+24
-19
lines changed

4 files changed

+24
-19
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.

test/pummel/test-heapdump-shadow-realm.js

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -30,10 +30,12 @@ function createRealms() {
3030
}
3131

3232
function validateHeap() {
33-
validateByRetainingPath('Node / Environment', [
34-
{ node_name: 'Node / shadow_realms', edge_name: 'shadow_realms' },
35-
{ node_name: 'Node / ShadowRealm' },
36-
]);
33+
// Validate that ShadowRealm appears in the heap snapshot.
34+
// We don't validate a specific retaining path since ShadowRealms are now
35+
// managed via weak references and cleanup hooks rather than a strong
36+
// shadow_realms_ set (which was removed to fix the memory leak).
37+
const nodes = validateByRetainingPath('Node / ShadowRealm', []);
38+
assert(nodes.length > 0, 'Expected at least one ShadowRealm in heap snapshot');
3739
}
3840

3941
createRealms();

0 commit comments

Comments
 (0)