Skip to content

src: clear all linked module caches once instantiated #59117

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Jul 29, 2025
Merged

src: clear all linked module caches once instantiated #59117

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
7886911
src: clear all linked module caches once instantiated
legendecas Jul 18, 2025
0313c1f
fixup! src: clear all linked module caches once instantiated
legendecas Jul 27, 2025
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
174 changes: 96 additions & 78 deletions src/module_wrap.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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() {
Expand All @@ -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());
Expand Down Expand Up @@ -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) {
Expand All @@ -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());
Expand Down Expand Up @@ -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());
Expand Down Expand Up @@ -965,48 +988,51 @@ 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>();
}

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>();
ModuleWrap* resolved_module;
if (!ResolveModule(context, specifier, import_attributes, referrer)
.To(&resolved_module)) {
return {};
}
DCHECK_NOT_NULL(resolved_module);
return resolved_module->module_.Get(context->GetIsolate());
}

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>();
// 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<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>();
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 {};
}

ModuleWrap* module;
ASSIGN_OR_RETURN_UNWRAP(&module, module_object, MaybeLocal<Module>());
return module->module_.Get(isolate);
CHECK(module_source_object->IsObject());
return module_source_object.As<Object>();
}

MaybeLocal<Object> ModuleWrap::ResolveSourceCallback(
// static
Maybe<ModuleWrap*> ModuleWrap::ResolveModule(
Local<Context> context,
Local<String> specifier,
Local<FixedArray> import_attributes,
Expand All @@ -1015,46 +1041,38 @@ MaybeLocal<Object> ModuleWrap::ResolveSourceCallback(
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->resolve_cache_.count(cache_key) != 1) {
if (!dependent->IsLinked()) {
THROW_ERR_VM_MODULE_LINK_FAILURE(
env, "request for '%s' is not in cache", cache_key.specifier);
return MaybeLocal<Object>();
env,
"request for '%s' is from a module not been linked",
cache_key.specifier);
return Nothing<ModuleWrap*>();
}

Local<Object> module_object =
dependent->resolve_cache_[cache_key].Get(isolate);
if (module_object.IsEmpty() || !module_object->IsObject()) {
auto it = dependent->resolve_cache_.find(cache_key);
Copy link
Member

@joyeecheung joyeecheung Jul 31, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I gave this a bit of thought and I think we might be able to just save the hash/look up cost completely if we have an V8 API that just always gives us the index. Put together a prototype: https://chromium-review.googlesource.com/c/v8/v8/+/6804466

if (it == dependent->resolve_cache_.end()) {
THROW_ERR_VM_MODULE_LINK_FAILURE(
env, "request for '%s' did not return an object", cache_key.specifier);
return MaybeLocal<Object>();
env, "request for '%s' is not in cache", cache_key.specifier);
return Nothing<ModuleWrap*>();
}

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(
Expand Down
28 changes: 20 additions & 8 deletions src/module_wrap.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
};

Expand All @@ -112,23 +116,25 @@ 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;
v8::Maybe<bool> CheckUnsettledTopLevelAwait();

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.
// Do these objects ever get GC'd? Are we just okay with leaking them?
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);

Expand Down Expand Up @@ -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_;
};

Expand Down
16 changes: 16 additions & 0 deletions test/parallel/test-internal-module-wrap.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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);

Expand All @@ -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());
Loading