Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 6 additions & 10 deletions src/env.cc
Original file line number Diff line number Diff line change
Expand Up @@ -259,13 +259,9 @@ 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);
}
// TrackShadowRealm/UntrackShadowRealm removed - they created strong references
// that prevented Shadow Realms from being garbage collected.
// Fixes: https://github.com/nodejs/node/issues/47353

AsyncHooks::DefaultTriggerAsyncIdScope::DefaultTriggerAsyncIdScope(
Environment* env, double default_trigger_async_id)
Expand Down Expand Up @@ -1044,8 +1040,8 @@ Environment::~Environment() {
inspector_agent_.reset();
#endif

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

if (trace_state_observer_) {
Expand Down Expand Up @@ -2232,7 +2228,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_);
// shadow_realms_ removed - no longer tracking to avoid strong references

// FIXME(joyeecheung): track other fields in Environment.
// Currently MemoryTracker is unable to track these
Expand Down
9 changes: 6 additions & 3 deletions src/env.h
Original file line number Diff line number Diff line change
Expand Up @@ -691,8 +691,9 @@ class Environment final : public MemoryRetainer {
Realm* realm,
const ContextInfo& info);
void UnassignFromContext(v8::Local<v8::Context> context);
void TrackShadowRealm(shadow_realm::ShadowRealm* realm);
void UntrackShadowRealm(shadow_realm::ShadowRealm* realm);
// TrackShadowRealm/UntrackShadowRealm removed - they created strong
// references that prevented Shadow Realms from being garbage collected.
// Fixes: https://github.com/nodejs/node/issues/47353

void StartProfilerIdleNotifier();

Expand Down Expand Up @@ -1117,7 +1118,9 @@ class Environment final : public MemoryRetainer {

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

#if HAVE_INSPECTOR
std::unique_ptr<profiler::V8CoverageConnection> coverage_connection_;
Expand Down
8 changes: 6 additions & 2 deletions src/node_shadow_realm.cc
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,10 @@ ShadowRealm::ShadowRealm(Environment* env)
context_.SetWeak(this, WeakCallback, v8::WeakCallbackType::kParameter);
CreateProperties();

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

Expand All @@ -88,7 +91,8 @@ ShadowRealm::~ShadowRealm() {
RunCleanup();
}

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

if (context_.IsEmpty()) {
// This most likely happened because the weak callback cleared it.
Expand Down
10 changes: 6 additions & 4 deletions test/pummel/test-heapdump-shadow-realm.js
Original file line number Diff line number Diff line change
Expand Up @@ -30,10 +30,12 @@ function createRealms() {
}

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

createRealms();