From 572d6b0643657595b7277de45927932694defc7e Mon Sep 17 00:00:00 2001 From: Chengzhong Wu Date: Wed, 13 Mar 2024 02:10:56 +0800 Subject: [PATCH] lib,src: iterate module requests of a module wrap in JS Avoid repetitively calling into JS callback from C++ in `ModuleWrap::Link`. This removes the convoluted callback style of the internal `ModuleWrap` link step. --- lib/internal/modules/esm/module_job.js | 84 +++--- lib/internal/vm/module.js | 92 ++++--- src/env_properties.h | 4 + src/module_wrap.cc | 247 ++++++------------ src/module_wrap.h | 9 +- .../es-module/test-esm-loader-concurrency.mjs | 51 ++++ test/parallel/test-internal-module-wrap.js | 12 +- 7 files changed, 247 insertions(+), 252 deletions(-) create mode 100644 test/es-module/test-esm-loader-concurrency.mjs diff --git a/lib/internal/modules/esm/module_job.js b/lib/internal/modules/esm/module_job.js index 43d9e141e04725..3a7400e3ca3a5a 100644 --- a/lib/internal/modules/esm/module_job.js +++ b/lib/internal/modules/esm/module_job.js @@ -1,8 +1,8 @@ 'use strict'; const { + Array, ArrayPrototypeJoin, - ArrayPrototypePush, ArrayPrototypeSome, FunctionPrototype, ObjectSetPrototypeOf, @@ -83,30 +83,8 @@ class ModuleJob extends ModuleJobBase { 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); @@ -116,6 +94,48 @@ class ModuleJob extends ModuleJobBase { 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); + + const moduleRequestsLength = this.module.moduleRequests.length; + // 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. + // Create an ArrayLike to avoid calling into userspace with `.then` + // when returned from the async function. + const dependencyJobs = Array(moduleRequestsLength); + ObjectSetPrototypeOf(dependencyJobs, null); + + // Specifiers should be aligned with the moduleRequests array in order. + const specifiers = Array(moduleRequestsLength); + const modulePromises = Array(moduleRequestsLength); + // Iterate with index to avoid calling into userspace with `Symbol.iterator`. + for (let idx = 0; idx < moduleRequestsLength; idx++) { + const { specifier, attributes } = this.module.moduleRequests[idx]; + + const dependencyJobPromise = this.loader.getModuleJob( + specifier, this.url, attributes, + ); + const modulePromise = PromisePrototypeThen(dependencyJobPromise, (job) => { + dependencyJobs[idx] = job; + return job.modulePromise; + }); + modulePromises[idx] = modulePromise; + specifiers[idx] = specifier; + } + + const modules = await SafePromiseAllReturnArrayLike(modulePromises); + this.module.link(specifiers, modules); + + return dependencyJobs; + } + instantiate() { if (this.instantiated === undefined) { this.instantiated = this._instantiate(); @@ -268,15 +288,17 @@ class ModuleJobSync extends ModuleJobBase { constructor(loader, url, importAttributes, moduleWrap, isMain, inspectBrk) { super(loader, url, importAttributes, moduleWrap, isMain, inspectBrk, true); assert(this.module instanceof ModuleWrap); - const moduleRequests = this.module.getModuleRequestsSync(); - for (let i = 0; i < moduleRequests.length; ++i) { - const { 0: specifier, 1: attributes } = moduleRequests[i]; + const moduleRequestsLength = this.module.moduleRequests.length; + // Specifiers should be aligned with the moduleRequests array in order. + const specifiers = Array(moduleRequestsLength); + const modules = Array(moduleRequestsLength); + for (let i = 0; i < moduleRequestsLength; ++i) { + const { specifier, attributes } = this.module.moduleRequests[i]; const wrap = this.loader.getModuleWrapForRequire(specifier, url, attributes); - const isLast = (i === moduleRequests.length - 1); - // TODO(joyeecheung): make the resolution callback deal with both promisified - // an raw module wraps, then we don't need to wrap it with a promise here. - this.module.cacheResolvedWrapsSync(specifier, PromiseResolve(wrap), isLast); + specifiers[i] = specifier; + modules[i] = wrap; } + this.module.link(specifiers, modules); } async run() { diff --git a/lib/internal/vm/module.js b/lib/internal/vm/module.js index c2ab2f258584da..6c8425ef155fa4 100644 --- a/lib/internal/vm/module.js +++ b/lib/internal/vm/module.js @@ -2,16 +2,20 @@ const assert = require('internal/assert'); const { + Array, ArrayIsArray, ArrayPrototypeForEach, ArrayPrototypeIndexOf, + ArrayPrototypeMap, ArrayPrototypeSome, ObjectDefineProperty, ObjectFreeze, ObjectGetPrototypeOf, ObjectSetPrototypeOf, + PromiseResolve, + PromisePrototypeThen, ReflectApply, - SafePromiseAllReturnVoid, + SafePromiseAllReturnArrayLike, Symbol, SymbolToStringTag, TypeError, @@ -303,46 +307,62 @@ class SourceTextModule extends Module { importModuleDynamically, }); - this[kLink] = async (linker) => { - this.#statusOverride = 'linking'; + this[kDependencySpecifiers] = undefined; + } - 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]; + async [kLink](linker) { + this.#statusOverride = 'linking'; + + const moduleRequestsLength = this[kWrap].moduleRequests.length; + // Iterates the module requests and links with the linker. + // Specifiers should be aligned with the moduleRequests array in order. + const specifiers = Array(moduleRequestsLength); + const modulePromises = Array(moduleRequestsLength); + // Iterates with index to avoid calling into userspace with `Symbol.iterator`. + for (let idx = 0; idx < moduleRequestsLength; idx++) { + const { specifier, attributes } = this[kWrap].moduleRequests[idx]; + + const linkerResult = linker(specifier, this, { + attributes, + assert: attributes, }); - - try { - if (promises !== undefined) { - await SafePromiseAllReturnVoid(promises); - } - } catch (e) { - this.#error = e; - throw e; - } finally { - this.#statusOverride = undefined; - } - }; - - this[kDependencySpecifiers] = undefined; + const modulePromise = 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]; + }); + modulePromises[idx] = modulePromise; + specifiers[idx] = specifier; + } + + try { + const modules = await SafePromiseAllReturnArrayLike(modulePromises); + this[kWrap].link(specifiers, modules); + } catch (e) { + this.#error = e; + throw e; + } finally { + this.#statusOverride = undefined; + } } get dependencySpecifiers() { 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]; } @@ -408,10 +428,10 @@ class SyntheticModule extends Module { context, identifier, }); + } - this[kLink] = () => this[kWrap].link(() => { - assert.fail('link callback should not be called'); - }); + [kLink]() { + /** nothing to do for synthetic modules */ } setExport(name, value) { diff --git a/src/env_properties.h b/src/env_properties.h index 05d8fcf576c51a..9d65b2a3290a84 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") \ @@ -377,6 +380,7 @@ V(js_transferable_constructor_template, v8::FunctionTemplate) \ V(libuv_stream_wrap_ctor_template, v8::FunctionTemplate) \ V(message_port_constructor_template, v8::FunctionTemplate) \ + V(module_wrap_constructor_template, v8::FunctionTemplate) \ V(microtask_queue_ctor_template, v8::FunctionTemplate) \ V(pipe_constructor_template, v8::FunctionTemplate) \ V(promise_wrap_template, v8::ObjectTemplate) \ diff --git a/src/module_wrap.cc b/src/module_wrap.cc index 92b34f5a8b99c2..c406bd2a5b7757 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,142 +361,35 @@ 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::GetModuleRequestsSync( - const FunctionCallbackInfo& args) { +// moduleWrap.link(specifiers, moduleWraps) +void ModuleWrap::Link(const FunctionCallbackInfo& args) { Realm* realm = Realm::GetCurrent(args); Isolate* isolate = args.GetIsolate(); - - Local that = args.This(); - - ModuleWrap* obj; - ASSIGN_OR_RETURN_UNWRAP(&obj, that); - - CHECK(!obj->linked_); - - Local module = obj->module_.Get(isolate); - Local module_requests = module->GetModuleRequests(); - const int module_requests_length = module_requests->Length(); - - std::vector> requests; - requests.reserve(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 raw_attributes = module_request->GetImportAssertions(); - std::vector> request = { - module_request->GetSpecifier(), - createImportAttributesContainer(realm, isolate, raw_attributes, 3), - }; - requests.push_back(Array::New(isolate, request.data(), request.size())); - } - - args.GetReturnValue().Set( - Array::New(isolate, requests.data(), requests.size())); -} - -void ModuleWrap::CacheResolvedWrapsSync( - const FunctionCallbackInfo& args) { - Isolate* isolate = args.GetIsolate(); - - CHECK_EQ(args.Length(), 3); - CHECK(args[0]->IsString()); - CHECK(args[1]->IsPromise()); - CHECK(args[2]->IsBoolean()); + Local context = realm->context(); ModuleWrap* dependent; ASSIGN_OR_RETURN_UNWRAP(&dependent, args.This()); - Utf8Value specifier(isolate, args[0]); - dependent->resolve_cache_[specifier.ToString()].Reset(isolate, - args[1].As()); - - if (args[2].As()->Value()) { - dependent->linked_ = true; - } -} - -void ModuleWrap::Link(const FunctionCallbackInfo& args) { - Realm* realm = Realm::GetCurrent(args); - Isolate* isolate = args.GetIsolate(); - - CHECK_EQ(args.Length(), 1); - CHECK(args[0]->IsFunction()); - - Local that = args.This(); - - 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); + CHECK_EQ(args.Length(), 2); - // 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 specifiers = args[0].As(); + Local modules = args[1].As(); + CHECK_EQ(specifiers->Length(), modules->Length()); - Local raw_attributes = module_request->GetImportAssertions(); - Local attributes = - createImportAttributesContainer(realm, isolate, raw_attributes, 3); + for (uint32_t i = 0; i < specifiers->Length(); i++) { + Local specifier_str = + specifiers->Get(context, i).ToLocalChecked().As(); + Local module_object = + modules->Get(context, i).ToLocalChecked().As(); - Local argv[] = { - specifier, - attributes, - }; + CHECK( + realm->isolate_data()->module_wrap_constructor_template()->HasInstance( + module_object)); - 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; + Utf8Value specifier(isolate, specifier_str); + dependent->resolve_cache_[specifier.ToString()].Reset(isolate, + module_object); } - - args.GetReturnValue().Set( - Array::New(isolate, promises.out(), promises.length())); } void ModuleWrap::Instantiate(const FunctionCallbackInfo& args) { @@ -700,29 +652,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; @@ -760,16 +689,8 @@ MaybeLocal ModuleWrap::ResolveModuleCallback( return MaybeLocal(); } - Local resolve_promise = + Local module_object = dependent->resolve_cache_[specifier_std].Get(isolate); - - if (resolve_promise->State() != Promise::kFulfilled) { - THROW_ERR_VM_MODULE_LINK_FAILURE( - env, "request for '%s' is not yet fulfilled", specifier_std); - return MaybeLocal(); - } - - Local module_object = resolve_promise->Result().As(); if (module_object.IsEmpty() || !module_object->IsObject()) { THROW_ERR_VM_MODULE_LINK_FAILURE( env, "request for '%s' did not return an object", specifier_std); @@ -996,9 +917,6 @@ void ModuleWrap::CreatePerIsolateProperties(IsolateData* isolate_data, ModuleWrap::kInternalFieldCount); SetProtoMethod(isolate, tpl, "link", Link); - SetProtoMethod(isolate, tpl, "getModuleRequestsSync", GetModuleRequestsSync); - SetProtoMethod( - isolate, tpl, "cacheResolvedWrapsSync", CacheResolvedWrapsSync); SetProtoMethod(isolate, tpl, "instantiateSync", InstantiateSync); SetProtoMethod(isolate, tpl, "evaluateSync", EvaluateSync); SetProtoMethod(isolate, tpl, "getNamespaceSync", GetNamespaceSync); @@ -1010,12 +928,8 @@ 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); + isolate_data->set_module_wrap_constructor_template(tpl); SetMethod(isolate, target, @@ -1053,8 +967,6 @@ void ModuleWrap::RegisterExternalReferences( registry->Register(New); registry->Register(Link); - registry->Register(GetModuleRequestsSync); - registry->Register(CacheResolvedWrapsSync); registry->Register(InstantiateSync); registry->Register(EvaluateSync); registry->Register(GetNamespaceSync); @@ -1065,7 +977,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 7c40ef08ea00f8..e08f57e1c74b22 100644 --- a/src/module_wrap.h +++ b/src/module_wrap.h @@ -79,10 +79,6 @@ class ModuleWrap : public BaseObject { ~ModuleWrap() override; static void New(const v8::FunctionCallbackInfo& args); - static void GetModuleRequestsSync( - const v8::FunctionCallbackInfo& args); - static void CacheResolvedWrapsSync( - const v8::FunctionCallbackInfo& args); static void InstantiateSync(const v8::FunctionCallbackInfo& args); static void EvaluateSync(const v8::FunctionCallbackInfo& args); static void GetNamespaceSync(const v8::FunctionCallbackInfo& args); @@ -93,8 +89,6 @@ class ModuleWrap : public BaseObject { 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); @@ -114,10 +108,9 @@ class ModuleWrap : public BaseObject { static ModuleWrap* GetFromModule(node::Environment*, v8::Local); v8::Global module_; - std::unordered_map> resolve_cache_; + std::unordered_map> resolve_cache_; contextify::ContextifyContext* contextify_context_ = nullptr; bool synthetic_ = false; - bool linked_ = false; int module_hash_; }; diff --git a/test/es-module/test-esm-loader-concurrency.mjs b/test/es-module/test-esm-loader-concurrency.mjs new file mode 100644 index 00000000000000..b3e270b9a22965 --- /dev/null +++ b/test/es-module/test-esm-loader-concurrency.mjs @@ -0,0 +1,51 @@ +import { spawnPromisified } from '../common/index.mjs'; +import * as fixtures from '../common/fixtures.mjs'; +import assert from 'node:assert'; +import { execPath } from 'node:process'; +import { describe, it } from 'node:test'; + +const setupArgs = [ + '--no-warnings', + '--input-type=module', + '--eval', +]; +const commonInput = ` +import os from "node:os"; +import url from "node:url"; +`; +const commonArgs = [ + ...setupArgs, + commonInput, +]; + +describe('ESM: loader concurrency', { concurrency: true }, () => { + it('should load module requests concurrently', async () => { + const { code, stderr, stdout } = await spawnPromisified( + execPath, + [ + '--loader', + fixtures.fileURL('es-module-loaders', 'loader-resolve-passthru.mjs'), + '--loader', + fixtures.fileURL('es-module-loaders', 'loader-load-passthru.mjs'), + ...commonArgs, + ], + { encoding: 'utf8' }, + ); + + console.log(stdout); + assert.strictEqual(stderr, ''); + // Verify that module requests are loaded concurrently without waiting for the previous one to finish. + // For example, if the module requests are loaded serially, the following output would be printed: + // ``` + // resolve passthru + // load passthru + // resolve passthru + // load passthru + // ``` + assert.match(stdout, new RegExp(`resolve passthru +resolve passthru +load passthru +load passthru`)); + assert.strictEqual(code, 0); + }); +}); diff --git a/test/parallel/test-internal-module-wrap.js b/test/parallel/test-internal-module-wrap.js index 520a83a3a47c0e..19f5ef911b0743 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.link(['bar'], [bar]); foo.instantiate(); assert.strictEqual(await foo.evaluate(-1, false), undefined);