diff --git a/lib/internal/modules/esm/module_job.js b/lib/internal/modules/esm/module_job.js index 108a8138443de0a..d7b6193d0bd1d56 100644 --- a/lib/internal/modules/esm/module_job.js +++ b/lib/internal/modules/esm/module_job.js @@ -78,30 +78,8 @@ class ModuleJob { this.modulePromise = PromiseResolve(this.modulePromise); } - // Wait for the ModuleWrap instance being linked with all dependencies. - const link = async () => { - this.module = await this.modulePromise; - assert(this.module instanceof ModuleWrap); - - // Explicitly keeping track of dependency jobs is needed in order - // to flatten out the dependency graph below in `_instantiate()`, - // so that circular dependencies can't cause a deadlock by two of - // these `link` callbacks depending on each other. - const dependencyJobs = []; - const promises = this.module.link(async (specifier, attributes) => { - const job = await this.loader.getModuleJob(specifier, url, attributes); - ArrayPrototypePush(dependencyJobs, job); - return job.modulePromise; - }); - - if (promises !== undefined) { - await SafePromiseAllReturnVoid(promises); - } - - return SafePromiseAllReturnArrayLike(dependencyJobs); - }; // Promise for the list of all dependencyJobs. - this.linked = link(); + this.linked = this._link(); // This promise is awaited later anyway, so silence // 'unhandled rejection' warnings. PromisePrototypeThen(this.linked, undefined, noop); @@ -111,6 +89,38 @@ class ModuleJob { this.instantiated = undefined; } + /** + * Iterates the module requests and links with the loader. + * @returns {Promise} Dependency module jobs. + */ + async _link() { + this.module = await this.modulePromise; + assert(this.module instanceof ModuleWrap); + + // Explicitly keeping track of dependency jobs is needed in order + // to flatten out the dependency graph below in `_instantiate()`, + // so that circular dependencies can't cause a deadlock by two of + // these `link` callbacks depending on each other. + const dependencyJobs = []; + const promises = []; + // Iterate with index to avoid calling into userspace with `Symbol.iterator`. + const moduleRequestsLength = this.module.moduleRequests.length; + for (let idx = 0; idx < moduleRequestsLength; idx++) { + const { specifier, attributes } = this.module.moduleRequests[idx]; + const job = await this.loader.getModuleJob(specifier, this.url, attributes); + ArrayPrototypePush(dependencyJobs, job); + ArrayPrototypePush(promises, job.modulePromise); + this.module.linkModule( + specifier, + job.modulePromise, + /** finished */ idx === moduleRequestsLength - 1, + ); + } + + await SafePromiseAllReturnVoid(promises); + return SafePromiseAllReturnArrayLike(dependencyJobs); + } + instantiate() { if (this.instantiated === undefined) { this.instantiated = this._instantiate(); diff --git a/lib/internal/vm/module.js b/lib/internal/vm/module.js index c2ab2f258584da7..0280de6743e3313 100644 --- a/lib/internal/vm/module.js +++ b/lib/internal/vm/module.js @@ -5,11 +5,15 @@ const { ArrayIsArray, ArrayPrototypeForEach, ArrayPrototypeIndexOf, + ArrayPrototypeMap, + ArrayPrototypePush, ArrayPrototypeSome, ObjectDefineProperty, ObjectFreeze, ObjectGetPrototypeOf, ObjectSetPrototypeOf, + PromiseResolve, + PromisePrototypeThen, ReflectApply, SafePromiseAllReturnVoid, Symbol, @@ -306,27 +310,40 @@ class SourceTextModule extends Module { this[kLink] = async (linker) => { this.#statusOverride = 'linking'; - const promises = this[kWrap].link(async (identifier, attributes) => { - const module = await linker(identifier, this, { attributes, assert: attributes }); - if (module[kWrap] === undefined) { - throw new ERR_VM_MODULE_NOT_MODULE(); - } - if (module.context !== this.context) { - throw new ERR_VM_MODULE_DIFFERENT_CONTEXT(); - } - if (module.status === 'errored') { - throw new ERR_VM_MODULE_LINK_FAILURE(`request for '${identifier}' resolved to an errored module`, module.error); - } - if (module.status === 'unlinked') { - await module[kLink](linker); - } - return module[kWrap]; - }); + // Iterates the module requests and links with the linker. + const promises = []; + // Iterate with index to avoid calling into userspace with `Symbol.iterator`. + const moduleRequestsLength = this[kWrap].moduleRequests.length; + for (let idx = 0; idx < moduleRequestsLength; idx++) { + const { specifier, attributes } = this[kWrap].moduleRequests[idx]; + + const linkerResult = linker(specifier, this, { + attributes, + assert: attributes, + }); + const promise = PromisePrototypeThen( + PromiseResolve(linkerResult), async (module) => { + if (module[kWrap] === undefined) { + throw new ERR_VM_MODULE_NOT_MODULE(); + } + if (module.context !== this.context) { + throw new ERR_VM_MODULE_DIFFERENT_CONTEXT(); + } + if (module.status === 'errored') { + throw new ERR_VM_MODULE_LINK_FAILURE(`request for '${specifier}' resolved to an errored module`, module.error); + } + if (module.status === 'unlinked') { + await module[kLink](linker); + } + return module[kWrap]; + }); + + this[kWrap].linkModule(specifier, promise, /** finished */ idx === moduleRequestsLength - 1); + ArrayPrototypePush(promises, promise); + } try { - if (promises !== undefined) { - await SafePromiseAllReturnVoid(promises); - } + await SafePromiseAllReturnVoid(promises); } catch (e) { this.#error = e; throw e; @@ -342,7 +359,8 @@ class SourceTextModule extends Module { if (this[kWrap] === undefined) { throw new ERR_VM_MODULE_NOT_MODULE(); } - this[kDependencySpecifiers] ??= ObjectFreeze(this[kWrap].getStaticDependencySpecifiers()); + this[kDependencySpecifiers] ??= ObjectFreeze( + ArrayPrototypeMap(this[kWrap].moduleRequests, (request) => request.specifier)); return this[kDependencySpecifiers]; } @@ -409,9 +427,7 @@ class SyntheticModule extends Module { identifier, }); - this[kLink] = () => this[kWrap].link(() => { - assert.fail('link callback should not be called'); - }); + this[kLink] = () => this[kWrap].markLinked(); } setExport(name, value) { diff --git a/src/env_properties.h b/src/env_properties.h index 82d2a64e88e1142..7f98feb761ef14b 100644 --- a/src/env_properties.h +++ b/src/env_properties.h @@ -67,6 +67,7 @@ V(args_string, "args") \ V(asn1curve_string, "asn1Curve") \ V(async_ids_stack_string, "async_ids_stack") \ + V(attributes_string, "attributes") \ V(base_string, "base") \ V(bits_string, "bits") \ V(block_list_string, "blockList") \ @@ -213,6 +214,7 @@ V(mgf1_hash_algorithm_string, "mgf1HashAlgorithm") \ V(minttl_string, "minttl") \ V(module_string, "module") \ + V(module_requests_string, "moduleRequests") \ V(modulus_string, "modulus") \ V(modulus_length_string, "modulusLength") \ V(name_string, "name") \ @@ -300,6 +302,7 @@ V(sni_context_string, "sni_context") \ V(source_string, "source") \ V(source_map_url_string, "sourceMapURL") \ + V(specifier_string, "specifier") \ V(stack_string, "stack") \ V(standard_name_string, "standardName") \ V(start_time_string, "startTime") \ diff --git a/src/module_wrap.cc b/src/module_wrap.cc index 25eb79d450c8074..ee8f5c9c50677fc 100644 --- a/src/module_wrap.cc +++ b/src/module_wrap.cc @@ -142,6 +142,57 @@ v8::Maybe ModuleWrap::CheckUnsettledTopLevelAwait() { return v8::Just(false); } +static Local createImportAttributesContainer( + Realm* realm, + Isolate* isolate, + Local raw_attributes, + const int elements_per_attribute) { + CHECK_EQ(raw_attributes->Length() % elements_per_attribute, 0); + Local attributes = + Object::New(isolate, v8::Null(isolate), nullptr, nullptr, 0); + for (int i = 0; i < raw_attributes->Length(); i += elements_per_attribute) { + attributes + ->Set(realm->context(), + raw_attributes->Get(realm->context(), i).As(), + raw_attributes->Get(realm->context(), i + 1).As()) + .ToChecked(); + } + + return attributes; +} + +static Local createModuleRequestsContainer( + Realm* realm, Isolate* isolate, Local raw_requests) { + Local requests = Array::New(isolate, raw_requests->Length()); + for (int i = 0; i < raw_requests->Length(); i++) { + Local module_request = + raw_requests->Get(realm->context(), i).As(); + + Local specifier = module_request->GetSpecifier(); + + Local raw_attributes = module_request->GetImportAssertions(); + Local attributes = + createImportAttributesContainer(realm, isolate, raw_attributes, 3); + + Local names[] = { + realm->isolate_data()->specifier_string(), + realm->isolate_data()->attributes_string(), + }; + Local values[] = { + specifier, + attributes, + }; + DCHECK_EQ(arraysize(names), arraysize(values)); + + Local request = Object::New( + isolate, v8::Null(isolate), names, values, arraysize(names)); + + requests->Set(realm->context(), i, request).ToChecked(); + } + + return requests; +} + // new ModuleWrap(url, context, source, lineOffset, columnOffset) // new ModuleWrap(url, context, exportNames, syntheticExecutionFunction) void ModuleWrap::New(const FunctionCallbackInfo& args) { @@ -279,6 +330,14 @@ void ModuleWrap::New(const FunctionCallbackInfo& args) { return; } + if (!that->Set(context, + realm->isolate_data()->module_requests_string(), + createModuleRequestsContainer( + realm, isolate, module->GetModuleRequests())) + .FromMaybe(false)) { + return; + } + if (that->SetPrivate(context, realm->isolate_data()->host_defined_option_symbol(), id_symbol) @@ -302,87 +361,34 @@ void ModuleWrap::New(const FunctionCallbackInfo& args) { args.GetReturnValue().Set(that); } -static Local createImportAttributesContainer( - Realm* realm, - Isolate* isolate, - Local raw_attributes, - const int elements_per_attribute) { - CHECK_EQ(raw_attributes->Length() % elements_per_attribute, 0); - Local attributes = - Object::New(isolate, v8::Null(isolate), nullptr, nullptr, 0); - for (int i = 0; i < raw_attributes->Length(); i += elements_per_attribute) { - attributes - ->Set(realm->context(), - raw_attributes->Get(realm->context(), i).As(), - raw_attributes->Get(realm->context(), i + 1).As()) - .ToChecked(); - } - - return attributes; -} - -void ModuleWrap::Link(const FunctionCallbackInfo& args) { - Realm* realm = Realm::GetCurrent(args); +// moduleWrap.linkModule(specifier, promise, finishedLink) +void ModuleWrap::LinkModule(const FunctionCallbackInfo& args) { Isolate* isolate = args.GetIsolate(); - CHECK_EQ(args.Length(), 1); - CHECK(args[0]->IsFunction()); - - Local that = args.This(); + CHECK_EQ(args.Length(), 3); + CHECK(args[0]->IsString()); + CHECK(args[1]->IsPromise()); + CHECK(args[2]->IsBoolean()); ModuleWrap* obj; - ASSIGN_OR_RETURN_UNWRAP(&obj, that); - - if (obj->linked_) - return; - obj->linked_ = true; - - Local resolver_arg = args[0].As(); - - Local mod_context = obj->context(); - Local module = obj->module_.Get(isolate); - - Local module_requests = module->GetModuleRequests(); - const int module_requests_length = module_requests->Length(); - MaybeStackBuffer, 16> promises(module_requests_length); - - // call the dependency resolve callbacks - for (int i = 0; i < module_requests_length; i++) { - Local module_request = - module_requests->Get(realm->context(), i).As(); - Local specifier = module_request->GetSpecifier(); - Utf8Value specifier_utf8(realm->isolate(), specifier); - std::string specifier_std(*specifier_utf8, specifier_utf8.length()); - - Local raw_attributes = module_request->GetImportAssertions(); - Local attributes = - createImportAttributesContainer(realm, isolate, raw_attributes, 3); + ASSIGN_OR_RETURN_UNWRAP(&obj, args.This()); + CHECK(!obj->linked_); - Local argv[] = { - specifier, - attributes, - }; + Utf8Value specifier(isolate, args[0]); + obj->resolve_cache_[specifier.ToString()].Reset(isolate, + args[1].As()); - MaybeLocal maybe_resolve_return_value = - resolver_arg->Call(mod_context, that, arraysize(argv), argv); - if (maybe_resolve_return_value.IsEmpty()) { - return; - } - Local resolve_return_value = - maybe_resolve_return_value.ToLocalChecked(); - if (!resolve_return_value->IsPromise()) { - THROW_ERR_VM_MODULE_LINK_FAILURE( - realm, "request for '%s' did not return promise", specifier_std); - return; - } - Local resolve_promise = resolve_return_value.As(); - obj->resolve_cache_[specifier_std].Reset(isolate, resolve_promise); - - promises[i] = resolve_promise; + if (args[2].As()->Value()) { + obj->linked_ = true; } +} - args.GetReturnValue().Set( - Array::New(isolate, promises.out(), promises.length())); +// Short cut for synthetic module to mark as linked. +void ModuleWrap::MarkLinked(const FunctionCallbackInfo& args) { + ModuleWrap* obj; + ASSIGN_OR_RETURN_UNWRAP(&obj, args.This()); + CHECK(!obj->linked_); + obj->linked_ = true; } void ModuleWrap::Instantiate(const FunctionCallbackInfo& args) { @@ -522,29 +528,6 @@ void ModuleWrap::GetStatus(const FunctionCallbackInfo& args) { args.GetReturnValue().Set(module->GetStatus()); } -void ModuleWrap::GetStaticDependencySpecifiers( - const FunctionCallbackInfo& args) { - Realm* realm = Realm::GetCurrent(args); - ModuleWrap* obj; - ASSIGN_OR_RETURN_UNWRAP(&obj, args.This()); - - Local module = obj->module_.Get(realm->isolate()); - - Local module_requests = module->GetModuleRequests(); - int count = module_requests->Length(); - - MaybeStackBuffer, 16> specifiers(count); - - for (int i = 0; i < count; i++) { - Local module_request = - module_requests->Get(realm->context(), i).As(); - specifiers[i] = module_request->GetSpecifier(); - } - - args.GetReturnValue().Set( - Array::New(realm->isolate(), specifiers.out(), count)); -} - void ModuleWrap::GetError(const FunctionCallbackInfo& args) { Isolate* isolate = args.GetIsolate(); ModuleWrap* obj; @@ -817,7 +800,8 @@ void ModuleWrap::CreatePerIsolateProperties(IsolateData* isolate_data, tpl->InstanceTemplate()->SetInternalFieldCount( ModuleWrap::kInternalFieldCount); - SetProtoMethod(isolate, tpl, "link", Link); + SetProtoMethod(isolate, tpl, "linkModule", LinkModule); + SetProtoMethod(isolate, tpl, "markLinked", MarkLinked); SetProtoMethod(isolate, tpl, "instantiate", Instantiate); SetProtoMethod(isolate, tpl, "evaluate", Evaluate); SetProtoMethod(isolate, tpl, "setExport", SetSyntheticExport); @@ -826,11 +810,6 @@ void ModuleWrap::CreatePerIsolateProperties(IsolateData* isolate_data, SetProtoMethodNoSideEffect(isolate, tpl, "getNamespace", GetNamespace); SetProtoMethodNoSideEffect(isolate, tpl, "getStatus", GetStatus); SetProtoMethodNoSideEffect(isolate, tpl, "getError", GetError); - SetProtoMethodNoSideEffect(isolate, - tpl, - "getStaticDependencySpecifiers", - GetStaticDependencySpecifiers); - SetConstructorFunction(isolate, target, "ModuleWrap", tpl); SetMethod(isolate, @@ -868,7 +847,8 @@ void ModuleWrap::RegisterExternalReferences( ExternalReferenceRegistry* registry) { registry->Register(New); - registry->Register(Link); + registry->Register(LinkModule); + registry->Register(MarkLinked); registry->Register(Instantiate); registry->Register(Evaluate); registry->Register(SetSyntheticExport); @@ -876,7 +856,6 @@ void ModuleWrap::RegisterExternalReferences( registry->Register(GetNamespace); registry->Register(GetStatus); registry->Register(GetError); - registry->Register(GetStaticDependencySpecifiers); registry->Register(SetImportModuleDynamicallyCallback); registry->Register(SetInitializeImportMetaObjectCallback); diff --git a/src/module_wrap.h b/src/module_wrap.h index 6f44d722ee0b013..7d430b4dfadd694 100644 --- a/src/module_wrap.h +++ b/src/module_wrap.h @@ -79,14 +79,13 @@ class ModuleWrap : public BaseObject { ~ModuleWrap() override; static void New(const v8::FunctionCallbackInfo& args); - static void Link(const v8::FunctionCallbackInfo& args); + static void LinkModule(const v8::FunctionCallbackInfo& args); + static void MarkLinked(const v8::FunctionCallbackInfo& args); static void Instantiate(const v8::FunctionCallbackInfo& args); static void Evaluate(const v8::FunctionCallbackInfo& args); static void GetNamespace(const v8::FunctionCallbackInfo& args); static void GetStatus(const v8::FunctionCallbackInfo& args); static void GetError(const v8::FunctionCallbackInfo& args); - static void GetStaticDependencySpecifiers( - const v8::FunctionCallbackInfo& args); static void SetImportModuleDynamicallyCallback( const v8::FunctionCallbackInfo& args); diff --git a/test/parallel/test-internal-module-wrap.js b/test/parallel/test-internal-module-wrap.js index 520a83a3a47c0e4..ffa73b1afd3b32c 100644 --- a/test/parallel/test-internal-module-wrap.js +++ b/test/parallel/test-internal-module-wrap.js @@ -5,21 +5,15 @@ const assert = require('assert'); const { internalBinding } = require('internal/test/binding'); const { ModuleWrap } = internalBinding('module_wrap'); -const { getPromiseDetails, isPromise } = internalBinding('util'); -const setTimeoutAsync = require('util').promisify(setTimeout); const foo = new ModuleWrap('foo', undefined, 'export * from "bar";', 0, 0); const bar = new ModuleWrap('bar', undefined, 'export const five = 5', 0, 0); (async () => { - const promises = foo.link(() => setTimeoutAsync(1000).then(() => bar)); - assert.strictEqual(promises.length, 1); - assert(isPromise(promises[0])); - - await Promise.all(promises); - - assert.strictEqual(getPromiseDetails(promises[0])[1], bar); + assert.strictEqual(foo.moduleRequests.length, 1); + assert.strictEqual(foo.moduleRequests[0].specifier, 'bar'); + foo.linkModule('bar', Promise.resolve(bar), true); foo.instantiate(); assert.strictEqual(await foo.evaluate(-1, false), undefined);