diff --git a/src/module_wrap.cc b/src/module_wrap.cc index 83728032406645..28843f6206e62a 100644 --- a/src/module_wrap.cc +++ b/src/module_wrap.cc @@ -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 = realm->context(); + Local requests = module->GetModuleRequests(); + for (int i = 0; i < requests->Length(); i++) { + ModuleCacheKey module_cache_key = ModuleCacheKey::From( + context, requests->Get(context, i).As()); + resolve_cache_[module_cache_key] = i; + } } ModuleWrap::~ModuleWrap() { @@ -159,6 +170,30 @@ Local ModuleWrap::context() const { return obj.As()->GetCreationContextChecked(); } +ModuleWrap* ModuleWrap::GetLinkedRequest(uint32_t index) { + DCHECK(IsLinked()); + Isolate* isolate = env()->isolate(); + EscapableHandleScope scope(isolate); + Local linked_requests_data = + object()->GetInternalField(kLinkedRequestsSlot); + DCHECK(linked_requests_data->IsValue() && + linked_requests_data.As()->IsArray()); + Local requests = linked_requests_data.As(); + + CHECK_LT(index, requests->Length()); + + Local module_value; + if (!requests->Get(context(), index).ToLocal(&module_value)) { + return nullptr; + } + CHECK(module_value->IsObject()); + Local module_object = module_value.As(); + + ModuleWrap* module_wrap; + ASSIGN_OR_RETURN_UNWRAP(&module_wrap, module_object, nullptr); + return module_wrap; +} + ModuleWrap* ModuleWrap::GetFromModule(Environment* env, Local module) { auto range = env->hash_to_module_map.equal_range(module->GetIdentityHash()); @@ -571,34 +606,28 @@ void ModuleWrap::GetModuleRequests(const FunctionCallbackInfo& args) { void ModuleWrap::Link(const FunctionCallbackInfo& args) { Realm* realm = Realm::GetCurrent(args); Isolate* isolate = args.GetIsolate(); - Local context = realm->context(); ModuleWrap* dependent; ASSIGN_OR_RETURN_UNWRAP(&dependent, args.This()); CHECK_EQ(args.Length(), 1); + Local linked_requests = + args.This()->GetInternalField(kLinkedRequestsSlot); + if (linked_requests->IsValue() && + !linked_requests.As()->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 requests = dependent->module_.Get(isolate)->GetModuleRequests(); Local modules = args[0].As(); CHECK_EQ(modules->Length(), static_cast(requests->Length())); - std::vector> modules_buffer; - if (FromV8Array(context, modules, &modules_buffer).IsNothing()) { - return; - } - - for (uint32_t i = 0; i < modules_buffer.size(); i++) { - Local module_object = modules_buffer[i].Get(isolate).As(); - - CHECK( - realm->isolate_data()->module_wrap_constructor_template()->HasInstance( - module_object)); - - ModuleCacheKey module_cache_key = ModuleCacheKey::From( - context, requests->Get(context, i).As()); - dependent->resolve_cache_[module_cache_key].Reset(isolate, module_object); - } + args.This()->SetInternalField(kLinkedRequestsSlot, modules); + dependent->linked_ = true; } void ModuleWrap::Instantiate(const FunctionCallbackInfo& args) { @@ -612,9 +641,6 @@ void ModuleWrap::Instantiate(const FunctionCallbackInfo& 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& 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,48 +988,51 @@ void ModuleWrap::GetError(const FunctionCallbackInfo& args) { args.GetReturnValue().Set(module->GetException()); } +// static MaybeLocal ModuleWrap::ResolveModuleCallback( Local context, Local specifier, Local import_attributes, Local referrer) { - Isolate* isolate = context->GetIsolate(); - Environment* env = Environment::GetCurrent(context); - if (env == nullptr) { - THROW_ERR_EXECUTION_ENVIRONMENT_NOT_AVAILABLE(isolate); - return MaybeLocal(); - } - - 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(); + 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(); +// static +MaybeLocal ModuleWrap::ResolveSourceCallback( + Local context, + Local specifier, + Local import_attributes, + Local referrer) { + ModuleWrap* resolved_module; + if (!ResolveModule(context, specifier, import_attributes, referrer) + .To(&resolved_module)) { + return {}; } + DCHECK_NOT_NULL(resolved_module); - Local 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(); + Local module_source_object = + resolved_module->object() + ->GetInternalField(ModuleWrap::kModuleSourceObjectSlot) + .As(); + if (module_source_object->IsUndefined()) { + Local url = resolved_module->object() + ->GetInternalField(ModuleWrap::kURLSlot) + .As(); + THROW_ERR_SOURCE_PHASE_NOT_DEFINED(context->GetIsolate(), url); + return {}; } - - ModuleWrap* module; - ASSIGN_OR_RETURN_UNWRAP(&module, module_object, MaybeLocal()); - return module->module_.Get(isolate); + CHECK(module_source_object->IsObject()); + return module_source_object.As(); } -MaybeLocal ModuleWrap::ResolveSourceCallback( +// static +Maybe ModuleWrap::ResolveModule( Local context, Local specifier, Local import_attributes, @@ -1015,46 +1041,38 @@ MaybeLocal ModuleWrap::ResolveSourceCallback( Environment* env = Environment::GetCurrent(context); if (env == nullptr) { THROW_ERR_EXECUTION_ENVIRONMENT_NOT_AVAILABLE(isolate); - return MaybeLocal(); + return Nothing(); } + // 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(); + return Nothing(); } - - 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(); + env, + "request for '%s' is from a module not been linked", + cache_key.specifier); + return Nothing(); } - Local module_object = - dependent->resolve_cache_[cache_key].Get(isolate); - if (module_object.IsEmpty() || !module_object->IsObject()) { + auto it = dependent->resolve_cache_.find(cache_key); + 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(); + env, "request for '%s' is not in cache", cache_key.specifier); + return Nothing(); } - ModuleWrap* module; - ASSIGN_OR_RETURN_UNWRAP(&module, module_object, MaybeLocal()); - - Local module_source_object = - module->object()->GetInternalField(kModuleSourceObjectSlot).As(); - if (module_source_object->IsUndefined()) { - Local url = - module->object()->GetInternalField(kURLSlot).As(); - THROW_ERR_SOURCE_PHASE_NOT_DEFINED(isolate, url); - return MaybeLocal(); - } - CHECK(module_source_object->IsObject()); - return module_source_object.As(); + ModuleWrap* module_wrap = dependent->GetLinkedRequest(it->second); + CHECK_NOT_NULL(module_wrap); + return Just(module_wrap); } static MaybeLocal ImportModuleDynamicallyWithPhase( diff --git a/src/module_wrap.h b/src/module_wrap.h index dbab9946c7210b..467a9af1177b0f 100644 --- a/src/module_wrap.h +++ b/src/module_wrap.h @@ -90,13 +90,17 @@ struct ModuleCacheKey : public MemoryRetainer { }; class ModuleWrap : public BaseObject { + using ResolveCache = + std::unordered_map; + 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 module, v8::Local meta); - void MemoryInfo(MemoryTracker* tracker) const override { - tracker->TrackField("resolve_cache", resolve_cache_); - } static void HasTopLevelAwait(const v8::FunctionCallbackInfo& args); v8::Local 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 GetHostDefinedOptions( v8::Isolate* isolate, v8::Local symbol); @@ -199,13 +205,19 @@ class ModuleWrap : public BaseObject { v8::Local referrer); static ModuleWrap* GetFromModule(node::Environment*, v8::Local); + // This method may throw a JavaScript exception, so the return type is + // wrapped in a Maybe. + static v8::Maybe ResolveModule( + v8::Local context, + v8::Local specifier, + v8::Local import_attributes, + v8::Local referrer); + v8::Global module_; - std::unordered_map, - ModuleCacheKey::Hash> - resolve_cache_; + ResolveCache resolve_cache_; contextify::ContextifyContext* contextify_context_ = nullptr; bool synthetic_ = false; + bool linked_ = false; int module_hash_; }; diff --git a/test/parallel/test-internal-module-wrap.js b/test/parallel/test-internal-module-wrap.js index 760d717c69ef8c..bed940d1368fa7 100644 --- a/test/parallel/test-internal-module-wrap.js +++ b/test/parallel/test-internal-module-wrap.js @@ -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());