src: clear all linked module caches once instantiated

There are two phases in module linking: link, and instantiate. These
two operations are required to be separated to allow cyclic
dependencies.

`v8::Module::InstantiateModule` is only required to be invoked on the
root module. The global references created by `ModuleWrap::Link` are
only cleared at `ModuleWrap::Instantiate`. So the global references
created for depended modules are usually not cleared because
`ModuleWrap::Instantiate` is not invoked for each of depended modules,
and caused memory leak.

The change references the linked modules in an object internal slot.

This is not an issue for Node.js ESM support as these modules can not be
off-loaded. However, this could be outstanding for `vm.Module`.

PR-URL: https://github.com/nodejs/node/pull/59117
Fixes: https://github.com/nodejs/node/issues/50113
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
This commit is contained in:
Chengzhong Wu 2025-07-29 13:17:52 +01:00 committed by GitHub
parent f904fa77f8
commit e55e0a7a98
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
3 changed files with 136 additions and 90 deletions

View file

@ -133,12 +133,23 @@ ModuleWrap::ModuleWrap(Realm* realm,
object->SetInternalField(kSyntheticEvaluationStepsSlot,
synthetic_evaluation_step);
object->SetInternalField(kContextObjectSlot, context_object);
object->SetInternalField(kLinkedRequestsSlot,
v8::Undefined(realm->isolate()));
if (!synthetic_evaluation_step->IsUndefined()) {
synthetic_ = true;
}
MakeWeak();
module_.SetWeak();
HandleScope scope(realm->isolate());
Local<Context> context = realm->context();
Local<FixedArray> requests = module->GetModuleRequests();
for (int i = 0; i < requests->Length(); i++) {
ModuleCacheKey module_cache_key = ModuleCacheKey::From(
context, requests->Get(context, i).As<ModuleRequest>());
resolve_cache_[module_cache_key] = i;
}
}
ModuleWrap::~ModuleWrap() {
@ -159,6 +170,30 @@ Local<Context> ModuleWrap::context() const {
return obj.As<Object>()->GetCreationContextChecked();
}
ModuleWrap* ModuleWrap::GetLinkedRequest(uint32_t index) {
DCHECK(IsLinked());
Isolate* isolate = env()->isolate();
EscapableHandleScope scope(isolate);
Local<Data> linked_requests_data =
object()->GetInternalField(kLinkedRequestsSlot);
DCHECK(linked_requests_data->IsValue() &&
linked_requests_data.As<Value>()->IsArray());
Local<Array> requests = linked_requests_data.As<Array>();
CHECK_LT(index, requests->Length());
Local<Value> module_value;
if (!requests->Get(context(), index).ToLocal(&module_value)) {
return nullptr;
}
CHECK(module_value->IsObject());
Local<Object> module_object = module_value.As<Object>();
ModuleWrap* module_wrap;
ASSIGN_OR_RETURN_UNWRAP(&module_wrap, module_object, nullptr);
return module_wrap;
}
ModuleWrap* ModuleWrap::GetFromModule(Environment* env,
Local<Module> module) {
auto range = env->hash_to_module_map.equal_range(module->GetIdentityHash());
@ -571,34 +606,28 @@ void ModuleWrap::GetModuleRequests(const FunctionCallbackInfo<Value>& args) {
void ModuleWrap::Link(const FunctionCallbackInfo<Value>& args) {
Realm* realm = Realm::GetCurrent(args);
Isolate* isolate = args.GetIsolate();
Local<Context> context = realm->context();
ModuleWrap* dependent;
ASSIGN_OR_RETURN_UNWRAP(&dependent, args.This());
CHECK_EQ(args.Length(), 1);
Local<Data> linked_requests =
args.This()->GetInternalField(kLinkedRequestsSlot);
if (linked_requests->IsValue() &&
!linked_requests.As<Value>()->IsUndefined()) {
// If the module is already linked, we should not link it again.
THROW_ERR_VM_MODULE_LINK_FAILURE(realm->env(), "module is already linked");
return;
}
Local<FixedArray> requests =
dependent->module_.Get(isolate)->GetModuleRequests();
Local<Array> modules = args[0].As<Array>();
CHECK_EQ(modules->Length(), static_cast<uint32_t>(requests->Length()));
std::vector<Global<Value>> modules_buffer;
if (FromV8Array(context, modules, &modules_buffer).IsNothing()) {
return;
}
for (uint32_t i = 0; i < modules_buffer.size(); i++) {
Local<Object> module_object = modules_buffer[i].Get(isolate).As<Object>();
CHECK(
realm->isolate_data()->module_wrap_constructor_template()->HasInstance(
module_object));
ModuleCacheKey module_cache_key = ModuleCacheKey::From(
context, requests->Get(context, i).As<ModuleRequest>());
dependent->resolve_cache_[module_cache_key].Reset(isolate, module_object);
}
args.This()->SetInternalField(kLinkedRequestsSlot, modules);
dependent->linked_ = true;
}
void ModuleWrap::Instantiate(const FunctionCallbackInfo<Value>& args) {
@ -612,9 +641,6 @@ void ModuleWrap::Instantiate(const FunctionCallbackInfo<Value>& args) {
USE(module->InstantiateModule(
context, ResolveModuleCallback, ResolveSourceCallback));
// clear resolve cache on instantiate
obj->resolve_cache_.clear();
if (try_catch.HasCaught() && !try_catch.HasTerminated()) {
CHECK(!try_catch.Message().IsEmpty());
CHECK(!try_catch.Exception().IsEmpty());
@ -722,9 +748,6 @@ void ModuleWrap::InstantiateSync(const FunctionCallbackInfo<Value>& args) {
USE(module->InstantiateModule(
context, ResolveModuleCallback, ResolveSourceCallback));
// clear resolve cache on instantiate
obj->resolve_cache_.clear();
if (try_catch.HasCaught() && !try_catch.HasTerminated()) {
CHECK(!try_catch.Message().IsEmpty());
CHECK(!try_catch.Exception().IsEmpty());
@ -965,96 +988,91 @@ void ModuleWrap::GetError(const FunctionCallbackInfo<Value>& args) {
args.GetReturnValue().Set(module->GetException());
}
// static
MaybeLocal<Module> ModuleWrap::ResolveModuleCallback(
Local<Context> context,
Local<String> specifier,
Local<FixedArray> import_attributes,
Local<Module> referrer) {
Isolate* isolate = context->GetIsolate();
Environment* env = Environment::GetCurrent(context);
if (env == nullptr) {
THROW_ERR_EXECUTION_ENVIRONMENT_NOT_AVAILABLE(isolate);
return MaybeLocal<Module>();
ModuleWrap* resolved_module;
if (!ResolveModule(context, specifier, import_attributes, referrer)
.To(&resolved_module)) {
return {};
}
ModuleCacheKey cache_key =
ModuleCacheKey::From(context, specifier, import_attributes);
ModuleWrap* dependent = GetFromModule(env, referrer);
if (dependent == nullptr) {
THROW_ERR_VM_MODULE_LINK_FAILURE(
env, "request for '%s' is from invalid module", cache_key.specifier);
return MaybeLocal<Module>();
}
if (dependent->resolve_cache_.count(cache_key) != 1) {
THROW_ERR_VM_MODULE_LINK_FAILURE(
env, "request for '%s' is not in cache", cache_key.specifier);
return MaybeLocal<Module>();
}
Local<Object> module_object =
dependent->resolve_cache_[cache_key].Get(isolate);
if (module_object.IsEmpty() || !module_object->IsObject()) {
THROW_ERR_VM_MODULE_LINK_FAILURE(
env, "request for '%s' did not return an object", cache_key.specifier);
return MaybeLocal<Module>();
}
ModuleWrap* module;
ASSIGN_OR_RETURN_UNWRAP(&module, module_object, MaybeLocal<Module>());
return module->module_.Get(isolate);
DCHECK_NOT_NULL(resolved_module);
return resolved_module->module_.Get(context->GetIsolate());
}
// static
MaybeLocal<Object> ModuleWrap::ResolveSourceCallback(
Local<Context> context,
Local<String> specifier,
Local<FixedArray> import_attributes,
Local<Module> referrer) {
ModuleWrap* resolved_module;
if (!ResolveModule(context, specifier, import_attributes, referrer)
.To(&resolved_module)) {
return {};
}
DCHECK_NOT_NULL(resolved_module);
Local<Value> module_source_object =
resolved_module->object()
->GetInternalField(ModuleWrap::kModuleSourceObjectSlot)
.As<Value>();
if (module_source_object->IsUndefined()) {
Local<String> url = resolved_module->object()
->GetInternalField(ModuleWrap::kURLSlot)
.As<String>();
THROW_ERR_SOURCE_PHASE_NOT_DEFINED(context->GetIsolate(), url);
return {};
}
CHECK(module_source_object->IsObject());
return module_source_object.As<Object>();
}
// static
Maybe<ModuleWrap*> ModuleWrap::ResolveModule(
Local<Context> context,
Local<String> specifier,
Local<FixedArray> import_attributes,
Local<Module> referrer) {
Isolate* isolate = context->GetIsolate();
Environment* env = Environment::GetCurrent(context);
if (env == nullptr) {
THROW_ERR_EXECUTION_ENVIRONMENT_NOT_AVAILABLE(isolate);
return MaybeLocal<Object>();
return Nothing<ModuleWrap*>();
}
// Check that the referrer is not yet been instantiated.
DCHECK(referrer->GetStatus() <= Module::kInstantiated);
ModuleCacheKey cache_key =
ModuleCacheKey::From(context, specifier, import_attributes);
ModuleWrap* dependent = GetFromModule(env, referrer);
ModuleWrap* dependent = ModuleWrap::GetFromModule(env, referrer);
if (dependent == nullptr) {
THROW_ERR_VM_MODULE_LINK_FAILURE(
env, "request for '%s' is from invalid module", cache_key.specifier);
return MaybeLocal<Object>();
return Nothing<ModuleWrap*>();
}
if (!dependent->IsLinked()) {
THROW_ERR_VM_MODULE_LINK_FAILURE(
env,
"request for '%s' is from a module not been linked",
cache_key.specifier);
return Nothing<ModuleWrap*>();
}
if (dependent->resolve_cache_.count(cache_key) != 1) {
auto it = dependent->resolve_cache_.find(cache_key);
if (it == dependent->resolve_cache_.end()) {
THROW_ERR_VM_MODULE_LINK_FAILURE(
env, "request for '%s' is not in cache", cache_key.specifier);
return MaybeLocal<Object>();
return Nothing<ModuleWrap*>();
}
Local<Object> module_object =
dependent->resolve_cache_[cache_key].Get(isolate);
if (module_object.IsEmpty() || !module_object->IsObject()) {
THROW_ERR_VM_MODULE_LINK_FAILURE(
env, "request for '%s' did not return an object", cache_key.specifier);
return MaybeLocal<Object>();
}
ModuleWrap* module;
ASSIGN_OR_RETURN_UNWRAP(&module, module_object, MaybeLocal<Object>());
Local<Value> module_source_object =
module->object()->GetInternalField(kModuleSourceObjectSlot).As<Value>();
if (module_source_object->IsUndefined()) {
Local<String> url =
module->object()->GetInternalField(kURLSlot).As<String>();
THROW_ERR_SOURCE_PHASE_NOT_DEFINED(isolate, url);
return MaybeLocal<Object>();
}
CHECK(module_source_object->IsObject());
return module_source_object.As<Object>();
ModuleWrap* module_wrap = dependent->GetLinkedRequest(it->second);
CHECK_NOT_NULL(module_wrap);
return Just(module_wrap);
}
static MaybeLocal<Promise> ImportModuleDynamicallyWithPhase(

View file

@ -90,13 +90,17 @@ struct ModuleCacheKey : public MemoryRetainer {
};
class ModuleWrap : public BaseObject {
using ResolveCache =
std::unordered_map<ModuleCacheKey, uint32_t, ModuleCacheKey::Hash>;
public:
enum InternalFields {
kModuleSlot = BaseObject::kInternalFieldCount,
kURLSlot,
kModuleSourceObjectSlot,
kSyntheticEvaluationStepsSlot,
kContextObjectSlot, // Object whose creation context is the target Context
kContextObjectSlot, // Object whose creation context is the target Context
kLinkedRequestsSlot, // Array of linked requests
kInternalFieldCount
};
@ -112,9 +116,6 @@ class ModuleWrap : public BaseObject {
v8::Local<v8::Module> module,
v8::Local<v8::Object> meta);
void MemoryInfo(MemoryTracker* tracker) const override {
tracker->TrackField("resolve_cache", resolve_cache_);
}
static void HasTopLevelAwait(const v8::FunctionCallbackInfo<v8::Value>& args);
v8::Local<v8::Context> context() const;
@ -122,6 +123,7 @@ class ModuleWrap : public BaseObject {
SET_MEMORY_INFO_NAME(ModuleWrap)
SET_SELF_SIZE(ModuleWrap)
SET_NO_MEMORY_INFO()
bool IsNotIndicativeOfMemoryLeakAtExit() const override {
// XXX: The garbage collection rules for ModuleWrap are *super* unclear.
@ -129,6 +131,10 @@ class ModuleWrap : public BaseObject {
return true;
}
bool IsLinked() const { return linked_; }
ModuleWrap* GetLinkedRequest(uint32_t index);
static v8::Local<v8::PrimitiveArray> GetHostDefinedOptions(
v8::Isolate* isolate, v8::Local<v8::Symbol> symbol);
@ -199,13 +205,19 @@ class ModuleWrap : public BaseObject {
v8::Local<v8::Module> referrer);
static ModuleWrap* GetFromModule(node::Environment*, v8::Local<v8::Module>);
// This method may throw a JavaScript exception, so the return type is
// wrapped in a Maybe.
static v8::Maybe<ModuleWrap*> ResolveModule(
v8::Local<v8::Context> context,
v8::Local<v8::String> specifier,
v8::Local<v8::FixedArray> import_attributes,
v8::Local<v8::Module> referrer);
v8::Global<v8::Module> module_;
std::unordered_map<ModuleCacheKey,
v8::Global<v8::Object>,
ModuleCacheKey::Hash>
resolve_cache_;
ResolveCache resolve_cache_;
contextify::ContextifyContext* contextify_context_ = nullptr;
bool synthetic_ = false;
bool linked_ = false;
int module_hash_;
};

View file

@ -6,6 +6,21 @@ const assert = require('assert');
const { internalBinding } = require('internal/test/binding');
const { ModuleWrap } = internalBinding('module_wrap');
const unlinked = new ModuleWrap('unlinked', undefined, 'export * from "bar";', 0, 0);
assert.throws(() => {
unlinked.instantiate();
}, {
code: 'ERR_VM_MODULE_LINK_FAILURE',
});
const dependsOnUnlinked = new ModuleWrap('dependsOnUnlinked', undefined, 'export * from "unlinked";', 0, 0);
dependsOnUnlinked.link([unlinked]);
assert.throws(() => {
dependsOnUnlinked.instantiate();
}, {
code: 'ERR_VM_MODULE_LINK_FAILURE',
});
const foo = new ModuleWrap('foo', undefined, 'export * from "bar";', 0, 0);
const bar = new ModuleWrap('bar', undefined, 'export const five = 5', 0, 0);
@ -22,4 +37,5 @@ const bar = new ModuleWrap('bar', undefined, 'export const five = 5', 0, 0);
// Check that the module requests are the same after linking, instantiate, and evaluation.
assert.deepStrictEqual(moduleRequests, foo.getModuleRequests());
})().then(common.mustCall());