diff --git a/doc/api/errors.md b/doc/api/errors.md index e2ce2c913eabb2..64c697df1c6dcb 100644 --- a/doc/api/errors.md +++ b/doc/api/errors.md @@ -2301,6 +2301,13 @@ The V8 platform used by this instance of Node.js does not support creating Workers. This is caused by lack of embedder support for Workers. In particular, this error will not occur with standard builds of Node.js. + + +### `ERR_MODULE_LINK_MISMATCH` + +A module can not be linked because the same module requests in it are not +resolved to the same module. + ### `ERR_MODULE_NOT_FOUND` diff --git a/doc/api/vm.md b/doc/api/vm.md index db4dad5b8cbe89..bf50aacaac43da 100644 --- a/doc/api/vm.md +++ b/doc/api/vm.md @@ -417,9 +417,7 @@ class that closely mirrors [Module Record][]s as defined in the ECMAScript specification. Unlike `vm.Script` however, every `vm.Module` object is bound to a context from -its creation. Operations on `vm.Module` objects are intrinsically asynchronous, -in contrast with the synchronous nature of `vm.Script` objects. The use of -'async' functions can help with manipulating `vm.Module` objects. +its creation. Using a `vm.Module` object requires three distinct steps: creation/parsing, linking, and evaluation. These three steps are illustrated in the following @@ -447,7 +445,7 @@ const contextifiedObject = vm.createContext({ // Here, we attempt to obtain the default export from the module "foo", and // put it into local binding "secret". -const bar = new vm.SourceTextModule(` +const rootModule = new vm.SourceTextModule(` import s from 'foo'; s; print(s); @@ -457,39 +455,48 @@ const bar = new vm.SourceTextModule(` // // "Link" the imported dependencies of this Module to it. // -// The provided linking callback (the "linker") accepts two arguments: the -// parent module (`bar` in this case) and the string that is the specifier of -// the imported module. The callback is expected to return a Module that -// corresponds to the provided specifier, with certain requirements documented -// in `module.link()`. -// -// If linking has not started for the returned Module, the same linker -// callback will be called on the returned Module. +// Obtain the requested dependencies of a SourceTextModule by +// `sourceTextModule.moduleRequests` and resolve them. // // Even top-level Modules without dependencies must be explicitly linked. The -// callback provided would never be called, however. -// -// The link() method returns a Promise that will be resolved when all the -// Promises returned by the linker resolve. +// array passed to `sourceTextModule.linkRequests(modules)` can be +// empty, however. // -// Note: This is a contrived example in that the linker function creates a new -// "foo" module every time it is called. In a full-fledged module system, a -// cache would probably be used to avoid duplicated modules. - -async function linker(specifier, referencingModule) { - if (specifier === 'foo') { - return new vm.SourceTextModule(` - // The "secret" variable refers to the global variable we added to - // "contextifiedObject" when creating the context. - export default secret; - `, { context: referencingModule.context }); - - // Using `contextifiedObject` instead of `referencingModule.context` - // here would work as well. - } - throw new Error(`Unable to resolve dependency: ${specifier}`); +// Note: This is a contrived example in that the resolveAndLinkDependencies +// creates a new "foo" module every time it is called. In a full-fledged +// module system, a cache would probably be used to avoid duplicated modules. + +const moduleMap = new Map([ + ['root', rootModule], +]); + +function resolveAndLinkDependencies(module) { + const requestedModules = module.moduleRequests.map((request) => { + // In a full-fledged module system, the resolveAndLinkDependencies would + // resolve the module with the module cache key `[specifier, attributes]`. + // In this example, we just use the specifier as the key. + const specifier = request.specifier; + + let requestedModule = moduleMap.get(specifier); + if (requestedModule === undefined) { + requestedModule = new vm.SourceTextModule(` + // The "secret" variable refers to the global variable we added to + // "contextifiedObject" when creating the context. + export default secret; + `, { context: referencingModule.context }); + moduleMap.set(specifier, linkedModule); + // Resolve the dependencies of the new module as well. + resolveAndLinkDependencies(requestedModule); + } + + return requestedModule; + }); + + module.linkRequests(requestedModules); } -await bar.link(linker); + +resolveAndLinkDependencies(rootModule); +rootModule.instantiate(); // Step 3 // @@ -497,7 +504,7 @@ await bar.link(linker); // resolve after the module has finished evaluating. // Prints 42. -await bar.evaluate(); +await rootModule.evaluate(); ``` ```cjs @@ -519,7 +526,7 @@ const contextifiedObject = vm.createContext({ // Here, we attempt to obtain the default export from the module "foo", and // put it into local binding "secret". - const bar = new vm.SourceTextModule(` + const rootModule = new vm.SourceTextModule(` import s from 'foo'; s; print(s); @@ -529,39 +536,48 @@ const contextifiedObject = vm.createContext({ // // "Link" the imported dependencies of this Module to it. // - // The provided linking callback (the "linker") accepts two arguments: the - // parent module (`bar` in this case) and the string that is the specifier of - // the imported module. The callback is expected to return a Module that - // corresponds to the provided specifier, with certain requirements documented - // in `module.link()`. - // - // If linking has not started for the returned Module, the same linker - // callback will be called on the returned Module. + // Obtain the requested dependencies of a SourceTextModule by + // `sourceTextModule.moduleRequests` and resolve them. // // Even top-level Modules without dependencies must be explicitly linked. The - // callback provided would never be called, however. - // - // The link() method returns a Promise that will be resolved when all the - // Promises returned by the linker resolve. + // array passed to `sourceTextModule.linkRequests(modules)` can be + // empty, however. // - // Note: This is a contrived example in that the linker function creates a new - // "foo" module every time it is called. In a full-fledged module system, a - // cache would probably be used to avoid duplicated modules. - - async function linker(specifier, referencingModule) { - if (specifier === 'foo') { - return new vm.SourceTextModule(` - // The "secret" variable refers to the global variable we added to - // "contextifiedObject" when creating the context. - export default secret; - `, { context: referencingModule.context }); + // Note: This is a contrived example in that the resolveAndLinkDependencies + // creates a new "foo" module every time it is called. In a full-fledged + // module system, a cache would probably be used to avoid duplicated modules. + + const moduleMap = new Map([ + ['root', rootModule], + ]); + + function resolveAndLinkDependencies(module) { + const requestedModules = module.moduleRequests.map((request) => { + // In a full-fledged module system, the resolveAndLinkDependencies would + // resolve the module with the module cache key `[specifier, attributes]`. + // In this example, we just use the specifier as the key. + const specifier = request.specifier; + + let requestedModule = moduleMap.get(specifier); + if (requestedModule === undefined) { + requestedModule = new vm.SourceTextModule(` + // The "secret" variable refers to the global variable we added to + // "contextifiedObject" when creating the context. + export default secret; + `, { context: referencingModule.context }); + moduleMap.set(specifier, linkedModule); + // Resolve the dependencies of the new module as well. + resolveAndLinkDependencies(requestedModule); + } + + return requestedModule; + }); - // Using `contextifiedObject` instead of `referencingModule.context` - // here would work as well. - } - throw new Error(`Unable to resolve dependency: ${specifier}`); + module.linkRequests(requestedModules); } - await bar.link(linker); + + resolveAndLinkDependencies(rootModule); + rootModule.instantiate(); // Step 3 // @@ -569,7 +585,7 @@ const contextifiedObject = vm.createContext({ // resolve after the module has finished evaluating. // Prints 42. - await bar.evaluate(); + await rootModule.evaluate(); })(); ``` @@ -658,6 +674,10 @@ changes: Link module dependencies. This method must be called before evaluation, and can only be called once per module. +Use [`sourceTextModule.linkRequests(modules)`][] and +[`sourceTextModule.instantiate()`][] to link modules either synchronously or +asynchronously. + The function is expected to return a `Module` object or a `Promise` that eventually resolves to a `Module` object. The returned `Module` must satisfy the following two invariants: @@ -803,8 +823,9 @@ const module = new vm.SourceTextModule( meta.prop = {}; }, }); -// Since module has no dependencies, the linker function will never be called. -await module.link(() => {}); +// The module has an empty `moduleRequests` array. +module.linkRequests([]); +module.instantiate(); await module.evaluate(); // Now, Object.prototype.secret will be equal to 42. @@ -830,8 +851,9 @@ const contextifiedObject = vm.createContext({ secret: 42 }); meta.prop = {}; }, }); - // Since module has no dependencies, the linker function will never be called. - await module.link(() => {}); + // The module has an empty `moduleRequests` array. + module.linkRequests([]); + module.instantiate(); await module.evaluate(); // Now, Object.prototype.secret will be equal to 42. // @@ -896,6 +918,69 @@ to disallow any changes to it. Corresponds to the `[[RequestedModules]]` field of [Cyclic Module Record][]s in the ECMAScript specification. +### `sourceTextModule.instantiate()` + + + +* Returns: {undefined} + +Instantiate the module with the linked requested modules. + +This resolves the imported bindings of the module, including re-exported +binding names. When there are any bindings that cannot be resolved, +an error would be thrown synchronously. + +If the requested modules include cyclic dependencies, the +[`sourceTextModule.linkRequests(modules)`][] method must be called on all +modules in the cycle before calling this method. + +### `sourceTextModule.linkRequests(modules)` + + + +* `modules` {vm.Module\[]} Array of `vm.Module` objects that this module depends on. + The order of the modules in the array is the order of + [`sourceTextModule.moduleRequests`][]. +* Returns: {undefined} + +Link module dependencies. This method must be called before evaluation, and +can only be called once per module. + +The order of the module instances in the `modules` array should correspond to the order of +[`sourceTextModule.moduleRequests`][] being resolved. If two module requests have the same +specifier and import attributes, they must be resolved with the same module instance or an +`ERR_MODULE_LINK_MISMATCH` would be thrown. For example, when linking requests for this +module: + + + +```mjs +import foo from 'foo'; +import source Foo from 'foo'; +``` + + + +The `modules` array must contain two references to the same instance, because the two +module requests are identical but in two phases. + +If the module has no dependencies, the `modules` array can be empty. + +Users can use `sourceTextModule.moduleRequests` to implement the host-defined +[HostLoadImportedModule][] abstract operation in the ECMAScript specification, +and using `sourceTextModule.linkRequests()` to invoke specification defined +[FinishLoadingImportedModule][], on the module with all dependencies in a batch. + +It's up to the creator of the `SourceTextModule` to determine if the resolution +of the dependencies is synchronous or asynchronous. + +After each module in the `modules` array is linked, call +[`sourceTextModule.instantiate()`][]. + ### `sourceTextModule.moduleRequests` * `name` {string} Name of the export to set. * `value` {any} The value to set the export to. -This method is used after the module is linked to set the values of exports. If -it is called before the module is linked, an [`ERR_VM_MODULE_STATUS`][] error -will be thrown. +This method sets the module export binding slots with the given value. ```mjs import vm from 'node:vm'; @@ -1021,7 +1109,6 @@ const m = new vm.SyntheticModule(['x'], () => { m.setExport('x', 1); }); -await m.link(() => {}); await m.evaluate(); assert.strictEqual(m.namespace.x, 1); @@ -1033,7 +1120,6 @@ const vm = require('node:vm'); const m = new vm.SyntheticModule(['x'], () => { m.setExport('x', 1); }); - await m.link(() => {}); await m.evaluate(); assert.strictEqual(m.namespace.x, 1); })(); @@ -2083,7 +2169,9 @@ const { Script, SyntheticModule } = require('node:vm'); [Cyclic Module Record]: https://tc39.es/ecma262/#sec-cyclic-module-records [ECMAScript Module Loader]: esm.md#modules-ecmascript-modules [Evaluate() concrete method]: https://tc39.es/ecma262/#sec-moduleevaluation +[FinishLoadingImportedModule]: https://tc39.es/ecma262/#sec-FinishLoadingImportedModule [GetModuleNamespace]: https://tc39.es/ecma262/#sec-getmodulenamespace +[HostLoadImportedModule]: https://tc39.es/ecma262/#sec-HostLoadImportedModule [HostResolveImportedModule]: https://tc39.es/ecma262/#sec-hostresolveimportedmodule [ImportDeclaration]: https://tc39.es/ecma262/#prod-ImportDeclaration [Link() concrete method]: https://tc39.es/ecma262/#sec-moduledeclarationlinking @@ -2095,13 +2183,14 @@ const { Script, SyntheticModule } = require('node:vm'); [WithClause]: https://tc39.es/ecma262/#prod-WithClause [`ERR_VM_DYNAMIC_IMPORT_CALLBACK_MISSING_FLAG`]: errors.md#err_vm_dynamic_import_callback_missing_flag [`ERR_VM_DYNAMIC_IMPORT_CALLBACK_MISSING`]: errors.md#err_vm_dynamic_import_callback_missing -[`ERR_VM_MODULE_STATUS`]: errors.md#err_vm_module_status [`Error`]: errors.md#class-error [`URL`]: url.md#class-url [`eval()`]: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/eval [`optionsExpression`]: https://tc39.es/proposal-import-attributes/#sec-evaluate-import-call [`script.runInContext()`]: #scriptrunincontextcontextifiedobject-options [`script.runInThisContext()`]: #scriptruninthiscontextoptions +[`sourceTextModule.instantiate()`]: #sourcetextmoduleinstantiate +[`sourceTextModule.linkRequests(modules)`]: #sourcetextmodulelinkrequestsmodules [`sourceTextModule.moduleRequests`]: #sourcetextmodulemodulerequests [`url.origin`]: url.md#urlorigin [`vm.compileFunction()`]: #vmcompilefunctioncode-params-options diff --git a/lib/internal/bootstrap/realm.js b/lib/internal/bootstrap/realm.js index c11f70dd6bf329..af283265d37dc4 100644 --- a/lib/internal/bootstrap/realm.js +++ b/lib/internal/bootstrap/realm.js @@ -360,6 +360,7 @@ class BuiltinModule { this.setExport('default', builtin.exports); }); // Ensure immediate sync execution to capture exports now + this.module.link([]); this.module.instantiate(); this.module.evaluate(-1, false); return this.module; diff --git a/lib/internal/errors.js b/lib/internal/errors.js index a9655974e673c3..e47f5d50918806 100644 --- a/lib/internal/errors.js +++ b/lib/internal/errors.js @@ -1592,6 +1592,7 @@ E('ERR_MISSING_ARGS', return `${msg} must be specified`; }, TypeError); E('ERR_MISSING_OPTION', '%s is required', TypeError); +E('ERR_MODULE_LINK_MISMATCH', '%s', TypeError); E('ERR_MODULE_NOT_FOUND', function(path, base, exactUrl) { if (exactUrl) { lazyInternalUtil().setOwnProperty(this, 'url', `${exactUrl}`); diff --git a/lib/internal/vm/module.js b/lib/internal/vm/module.js index 266e019be9c3da..e6e5c9d40749cc 100644 --- a/lib/internal/vm/module.js +++ b/lib/internal/vm/module.js @@ -38,6 +38,7 @@ const { ERR_VM_MODULE_DIFFERENT_CONTEXT, ERR_VM_MODULE_CANNOT_CREATE_CACHED_DATA, ERR_VM_MODULE_LINK_FAILURE, + ERR_MODULE_LINK_MISMATCH, ERR_VM_MODULE_NOT_MODULE, ERR_VM_MODULE_STATUS, } = require('internal/errors').codes; @@ -50,6 +51,7 @@ const { validateUint32, validateString, validateThisInternalField, + validateArray, } = require('internal/validators'); const binding = internalBinding('module_wrap'); @@ -354,6 +356,37 @@ class SourceTextModule extends Module { } } + linkRequests(modules) { + validateThisInternalField(this, kWrap, 'SourceTextModule'); + if (this.status !== 'unlinked') { + throw new ERR_VM_MODULE_STATUS('must be unlinked'); + } + validateArray(modules, 'modules'); + if (modules.length !== this.#moduleRequests.length) { + throw new ERR_MODULE_LINK_MISMATCH( + `Expected ${this.#moduleRequests.length} modules, got ${modules.length}`, + ); + } + const moduleWraps = ArrayPrototypeMap(modules, (module) => { + if (!isModule(module)) { + throw new ERR_VM_MODULE_NOT_MODULE(); + } + if (module.context !== this.context) { + throw new ERR_VM_MODULE_DIFFERENT_CONTEXT(); + } + return module[kWrap]; + }); + this[kWrap].link(moduleWraps); + } + + instantiate() { + validateThisInternalField(this, kWrap, 'SourceTextModule'); + if (this.status !== 'unlinked') { + throw new ERR_VM_MODULE_STATUS('must be unlinked'); + } + this[kWrap].instantiate(); + } + get dependencySpecifiers() { this.#dependencySpecifiers ??= ObjectFreeze( ArrayPrototypeMap(this.#moduleRequests, (request) => request.specifier)); @@ -420,10 +453,15 @@ class SyntheticModule extends Module { context, identifier, }); + // A synthetic module does not have dependencies. + this[kWrap].link([]); + this[kWrap].instantiate(); } - [kLink]() { - /** nothing to do for synthetic modules */ + link() { + validateThisInternalField(this, kWrap, 'SyntheticModule'); + // No-op for synthetic modules + // Do not invoke super.link() as it will throw an error. } setExport(name, value) { diff --git a/src/module_wrap.cc b/src/module_wrap.cc index 1f73ee09de51bf..1ff4971d6fedf6 100644 --- a/src/module_wrap.cc +++ b/src/module_wrap.cc @@ -67,6 +67,25 @@ void ModuleCacheKey::MemoryInfo(MemoryTracker* tracker) const { tracker->TrackField("import_attributes", import_attributes); } +std::string ModuleCacheKey::ToString() const { + std::string result = "ModuleCacheKey(\"" + specifier + "\""; + if (!import_attributes.empty()) { + result += ", {"; + bool first = true; + for (const auto& attr : import_attributes) { + if (first) { + first = false; + } else { + result += ", "; + } + result += attr.first + ": " + attr.second; + } + result += "}"; + } + result += ")"; + return result; +} + template ModuleCacheKey ModuleCacheKey::From(Local context, Local specifier, @@ -123,12 +142,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() { @@ -149,6 +179,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()); @@ -540,8 +594,8 @@ void ModuleWrap::GetModuleRequests(const FunctionCallbackInfo& args) { // moduleWrap.link(moduleWraps) void ModuleWrap::Link(const FunctionCallbackInfo& args) { - Realm* realm = Realm::GetCurrent(args); Isolate* isolate = args.GetIsolate(); + Realm* realm = Realm::GetCurrent(args); Local context = realm->context(); ModuleWrap* dependent; @@ -554,22 +608,32 @@ void ModuleWrap::Link(const FunctionCallbackInfo& args) { 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)); - + for (int i = 0; i < requests->Length(); i++) { ModuleCacheKey module_cache_key = ModuleCacheKey::From( context, requests->Get(context, i).As()); - dependent->resolve_cache_[module_cache_key].Reset(isolate, module_object); + DCHECK(dependent->resolve_cache_.contains(module_cache_key)); + + Local module_i; + Local module_cache_i; + uint32_t coalesced_index = dependent->resolve_cache_[module_cache_key]; + if (!modules->Get(context, i).ToLocal(&module_i) || + !modules->Get(context, coalesced_index).ToLocal(&module_cache_i) || + !module_i->StrictEquals(module_cache_i)) { + // If the module is different from the one of the same request, throw an + // error. + THROW_ERR_MODULE_LINK_MISMATCH( + realm->env(), + "Module request '%s' at index %d must be linked " + "to the same module requested at index %d", + module_cache_key.ToString(), + i, + coalesced_index); + return; + } } + + args.This()->SetInternalField(kLinkedRequestsSlot, modules); + dependent->linked_ = true; } void ModuleWrap::Instantiate(const FunctionCallbackInfo& args) { @@ -579,12 +643,15 @@ void ModuleWrap::Instantiate(const FunctionCallbackInfo& args) { ASSIGN_OR_RETURN_UNWRAP(&obj, args.This()); Local context = obj->context(); Local module = obj->module_.Get(isolate); + + if (!obj->IsLinked()) { + THROW_ERR_VM_MODULE_LINK_FAILURE(realm->env(), "module is not linked"); + return; + } + TryCatchScope try_catch(realm->env()); USE(module->InstantiateModule(context, ResolveModuleCallback)); - // 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()); @@ -731,9 +798,6 @@ void ModuleWrap::InstantiateSync(const FunctionCallbackInfo& args) { TryCatchScope try_catch(env); USE(module->InstantiateModule(context, ResolveModuleCallback)); - // 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()); @@ -940,45 +1004,63 @@ void ModuleWrap::GetError(const FunctionCallbackInfo& args) { args.GetReturnValue().Set(module->GetException()); } +// static MaybeLocal ModuleWrap::ResolveModuleCallback( 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); + return resolved_module->module_.Get(context->GetIsolate()); +} + +// static +Maybe ModuleWrap::ResolveModule( + 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(); + 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()); - return module->module_.Get(isolate); + ModuleWrap* module_wrap = dependent->GetLinkedRequest(it->second); + CHECK_NOT_NULL(module_wrap); + return Just(module_wrap); } static MaybeLocal ImportModuleDynamically( diff --git a/src/module_wrap.h b/src/module_wrap.h index ef0dcc0f6d1643..695b73ca7ffea8 100644 --- a/src/module_wrap.h +++ b/src/module_wrap.h @@ -53,6 +53,9 @@ struct ModuleCacheKey : public MemoryRetainer { SET_SELF_SIZE(ModuleCacheKey) void MemoryInfo(MemoryTracker* tracker) const override; + // Returns a string representation of the ModuleCacheKey. + std::string ToString() const; + template static ModuleCacheKey From(v8::Local context, v8::Local specifier, @@ -84,12 +87,16 @@ struct ModuleCacheKey : public MemoryRetainer { }; class ModuleWrap : public BaseObject { + using ResolveCache = + std::unordered_map; + public: enum InternalFields { kModuleSlot = BaseObject::kInternalFieldCount, kURLSlot, 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 }; @@ -105,9 +112,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; @@ -115,6 +119,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. @@ -122,6 +127,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); @@ -183,13 +192,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/src/node_errors.h b/src/node_errors.h index 8f370a66b48809..791e835c2c2851 100644 --- a/src/node_errors.h +++ b/src/node_errors.h @@ -109,6 +109,7 @@ void OOMErrorHandler(const char* location, const v8::OOMDetails& details); V(ERR_MISSING_PASSPHRASE, TypeError) \ V(ERR_MISSING_PLATFORM_FOR_WORKER, Error) \ V(ERR_MODULE_NOT_FOUND, Error) \ + V(ERR_MODULE_LINK_MISMATCH, TypeError) \ V(ERR_NON_CONTEXT_AWARE_DISABLED, Error) \ V(ERR_OPTIONS_BEFORE_BOOTSTRAPPING, Error) \ V(ERR_OUT_OF_RANGE, RangeError) \ diff --git a/test/parallel/test-internal-module-wrap.js b/test/parallel/test-internal-module-wrap.js index 760d717c69ef8c..e9c0119bdebd5d 100644 --- a/test/parallel/test-internal-module-wrap.js +++ b/test/parallel/test-internal-module-wrap.js @@ -6,10 +6,25 @@ const assert = require('assert'); const { internalBinding } = require('internal/test/binding'); const { ModuleWrap } = internalBinding('module_wrap'); -const foo = new ModuleWrap('foo', undefined, 'export * from "bar";', 0, 0); -const bar = new ModuleWrap('bar', undefined, 'export const five = 5', 0, 0); +async function testModuleWrap() { + 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); -(async () => { const moduleRequests = foo.getModuleRequests(); assert.strictEqual(moduleRequests.length, 1); assert.strictEqual(moduleRequests[0].specifier, 'bar'); @@ -22,4 +37,8 @@ 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()); +} + +(async () => { + await testModuleWrap(); })().then(common.mustCall()); diff --git a/test/parallel/test-vm-module-basic.js b/test/parallel/test-vm-module-basic.js index a8f5b92b7adc4b..f38bc39d70daec 100644 --- a/test/parallel/test-vm-module-basic.js +++ b/test/parallel/test-vm-module-basic.js @@ -99,7 +99,7 @@ const util = require('util'); assert.strictEqual( util.inspect(m), `SyntheticModule { - status: 'unlinked', + status: 'linked', identifier: 'vm:module(0)', context: { foo: 'bar' } }` diff --git a/test/parallel/test-vm-module-instantiate.js b/test/parallel/test-vm-module-instantiate.js new file mode 100644 index 00000000000000..e96a78a8f643d4 --- /dev/null +++ b/test/parallel/test-vm-module-instantiate.js @@ -0,0 +1,99 @@ +'use strict'; + +// Flags: --experimental-vm-modules + +require('../common'); + +const assert = require('assert'); + +const { SourceTextModule } = require('vm'); +const test = require('node:test'); + +test('simple module', () => { + const foo = new SourceTextModule(` + export const foo = 4 + export default 5; + `); + foo.linkRequests([]); + foo.instantiate(); + + assert.deepStrictEqual( + Reflect.ownKeys(foo.namespace), + ['default', 'foo', Symbol.toStringTag] + ); +}); + +test('linkRequests can not be skipped', () => { + const foo = new SourceTextModule(` + export const foo = 4 + export default 5; + `); + assert.throws(() => { + foo.instantiate(); + }, { + code: 'ERR_VM_MODULE_LINK_FAILURE', + }); +}); + +test('re-export simple name', () => { + const foo = new SourceTextModule(` + export { bar } from 'bar'; + `); + const bar = new SourceTextModule(` + export const bar = 42; + `); + foo.linkRequests([bar]); + foo.instantiate(); + + assert.deepStrictEqual( + Reflect.ownKeys(foo.namespace), + ['bar', Symbol.toStringTag] + ); +}); + +test('re-export-star', () => { + const foo = new SourceTextModule(` + export * from 'bar'; + `); + const bar = new SourceTextModule(` + export const bar = 42; + `); + foo.linkRequests([bar]); + foo.instantiate(); + + assert.deepStrictEqual( + Reflect.ownKeys(foo.namespace), + ['bar', Symbol.toStringTag] + ); +}); + +test('deep re-export-star', () => { + let stackTop = new SourceTextModule(` + export const foo = 4; + `); + stackTop.linkRequests([]); + for (let i = 0; i < 10; i++) { + const mod = new SourceTextModule(` + export * from 'stack?top'; + `); + mod.linkRequests([stackTop]); + stackTop = mod; + } + stackTop.instantiate(); + + assert.deepStrictEqual( + Reflect.ownKeys(stackTop.namespace), + ['foo', Symbol.toStringTag] + ); +}); + +test('should throw if the module is not linked', () => { + const foo = new SourceTextModule(` + import { bar } from 'bar'; + `); + assert.throws(() => { + foo.instantiate(); + }, { + code: 'ERR_VM_MODULE_LINK_FAILURE', + }); +}); diff --git a/test/parallel/test-vm-module-link-shared-deps.js b/test/parallel/test-vm-module-link-shared-deps.js new file mode 100644 index 00000000000000..4d4bae253a426d --- /dev/null +++ b/test/parallel/test-vm-module-link-shared-deps.js @@ -0,0 +1,82 @@ +// Flags: --experimental-vm-modules + +'use strict'; + +const common = require('../common'); +const vm = require('vm'); +const assert = require('assert'); + +// This test verifies that a module can be returned multiple +// times in the linker function in `module.link(linker)`. +// `module.link(linker)` should handle the race condition of +// `module.status` when the linker function is asynchronous. + +// Regression of https://github.com/nodejs/node/issues/59480 + +const sources = { + './index.js': ` + import foo from "./foo.js"; + import shared from "./shared.js"; + export default { + foo, + shared + }; + `, + './foo.js': ` + import shared from "./shared.js"; + export default { + name: "foo" + }; + `, + './shared.js': ` + export default { + name: "shared", + }; + `, +}; + +const moduleCache = new Map(); + +function getModuleInstance(identifier) { + let module = moduleCache.get(identifier); + + if (!module) { + module = new vm.SourceTextModule(sources[identifier], { + identifier, + }); + moduleCache.set(identifier, module); + } + + return module; +} + +async function esmImport(identifier) { + const module = getModuleInstance(identifier); + const requests = []; + + await module.link(async (specifier, referrer) => { + requests.push([specifier, referrer.identifier]); + // Use `Promise.resolve` to defer a tick to create a race condition on + // `module.status` when a module is being imported several times. + return Promise.resolve(getModuleInstance(specifier)); + }); + + await module.evaluate(); + return [module.namespace, requests]; +} + +async function test() { + const { 0: mod, 1: requests } = await esmImport('./index.js'); + assert.strictEqual(mod.default.foo.name, 'foo'); + assert.strictEqual(mod.default.shared.name, 'shared'); + + // Assert that there is no duplicated requests. + assert.deepStrictEqual(requests, [ + // [specifier, referrer] + ['./foo.js', './index.js'], + ['./shared.js', './index.js'], + ['./shared.js', './foo.js'], + ]); +} + +test().then(common.mustCall()); diff --git a/test/parallel/test-vm-module-linkmodulerequests-circular.js b/test/parallel/test-vm-module-linkmodulerequests-circular.js new file mode 100644 index 00000000000000..26489d1a1610e1 --- /dev/null +++ b/test/parallel/test-vm-module-linkmodulerequests-circular.js @@ -0,0 +1,72 @@ +'use strict'; + +// Flags: --experimental-vm-modules + +require('../common'); + +const assert = require('assert'); + +const { SourceTextModule } = require('vm'); +const test = require('node:test'); + +test('basic circular linking', async function circular() { + const foo = new SourceTextModule(` + import getFoo from 'bar'; + export let foo = 42; + export default getFoo(); + `); + const bar = new SourceTextModule(` + import { foo } from 'foo'; + export default function getFoo() { + return foo; + } + `); + foo.linkRequests([bar]); + bar.linkRequests([foo]); + + foo.instantiate(); + assert.strictEqual(foo.status, 'linked'); + assert.strictEqual(bar.status, 'linked'); + + await foo.evaluate(); + assert.strictEqual(foo.namespace.default, 42); +}); + +test('circular linking graph', async function circular2() { + const sourceMap = { + 'root': ` + import * as a from './a.mjs'; + import * as b from './b.mjs'; + if (!('fromA' in a)) + throw new Error(); + if (!('fromB' in a)) + throw new Error(); + if (!('fromA' in b)) + throw new Error(); + if (!('fromB' in b)) + throw new Error(); + `, + './a.mjs': ` + export * from './b.mjs'; + export let fromA; + `, + './b.mjs': ` + export * from './a.mjs'; + export let fromB; + ` + }; + const moduleMap = new Map(); + for (const [specifier, source] of Object.entries(sourceMap)) { + moduleMap.set(specifier, new SourceTextModule(source, { + identifier: new URL(specifier, 'file:///').href, + })); + } + for (const mod of moduleMap.values()) { + mod.linkRequests(mod.moduleRequests.map((request) => { + return moduleMap.get(request.specifier); + })); + } + const rootModule = moduleMap.get('root'); + rootModule.instantiate(); + await rootModule.evaluate(); +}); diff --git a/test/parallel/test-vm-module-linkmodulerequests-deep.js b/test/parallel/test-vm-module-linkmodulerequests-deep.js new file mode 100644 index 00000000000000..4d335d6edcb57d --- /dev/null +++ b/test/parallel/test-vm-module-linkmodulerequests-deep.js @@ -0,0 +1,34 @@ +'use strict'; + +// Flags: --experimental-vm-modules + +require('../common'); + +const assert = require('assert'); + +const { SourceTextModule } = require('vm'); +const test = require('node:test'); + +test('deep linking', async function depth() { + const foo = new SourceTextModule('export default 5'); + foo.linkRequests([]); + foo.instantiate(); + + function getProxy(parentName, parentModule) { + const mod = new SourceTextModule(` + import ${parentName} from '${parentName}'; + export default ${parentName}; + `); + mod.linkRequests([parentModule]); + mod.instantiate(); + return mod; + } + + const bar = getProxy('foo', foo); + const baz = getProxy('bar', bar); + const barz = getProxy('baz', baz); + + await barz.evaluate(); + + assert.strictEqual(barz.namespace.default, 5); +}); diff --git a/test/parallel/test-vm-module-linkmodulerequests.js b/test/parallel/test-vm-module-linkmodulerequests.js new file mode 100644 index 00000000000000..c6adeb7a0d84e2 --- /dev/null +++ b/test/parallel/test-vm-module-linkmodulerequests.js @@ -0,0 +1,58 @@ +'use strict'; + +// Flags: --experimental-vm-modules + +require('../common'); + +const assert = require('assert'); + +const { SourceTextModule } = require('vm'); +const test = require('node:test'); + +test('simple graph', async function simple() { + const foo = new SourceTextModule('export default 5;'); + foo.linkRequests([]); + + globalThis.fiveResult = undefined; + const bar = new SourceTextModule('import five from "foo"; fiveResult = five'); + + bar.linkRequests([foo]); + bar.instantiate(); + + await bar.evaluate(); + assert.strictEqual(globalThis.fiveResult, 5); + delete globalThis.fiveResult; +}); + +test('invalid link values', () => { + const invalidValues = [ + undefined, + null, + {}, + SourceTextModule.prototype, + ]; + + for (const value of invalidValues) { + const module = new SourceTextModule('import "foo"'); + assert.throws(() => module.linkRequests([value]), { + code: 'ERR_VM_MODULE_NOT_MODULE', + }); + } +}); + +test('mismatch linkage', () => { + const foo = new SourceTextModule('export default 5;'); + foo.linkRequests([]); + + // Link with more modules than requested. + const bar = new SourceTextModule('import foo from "foo";'); + assert.throws(() => bar.linkRequests([foo, foo]), { + code: 'ERR_MODULE_LINK_MISMATCH', + }); + + // Link with fewer modules than requested. + const baz = new SourceTextModule('import foo from "foo"; import bar from "bar";'); + assert.throws(() => baz.linkRequests([foo]), { + code: 'ERR_MODULE_LINK_MISMATCH', + }); +}); diff --git a/test/parallel/test-vm-module-modulerequests.js b/test/parallel/test-vm-module-modulerequests.js index ad5aa1518fc3b1..d7504a9e768eed 100644 --- a/test/parallel/test-vm-module-modulerequests.js +++ b/test/parallel/test-vm-module-modulerequests.js @@ -12,9 +12,13 @@ const test = require('node:test'); test('SourceTextModule.moduleRequests should return module requests', (t) => { const m = new SourceTextModule(` import { foo } from './foo.js'; + import * as FooDuplicate from './foo.js'; import { bar } from './bar.json' with { type: 'json' }; + import * as BarDuplicate from './bar.json' with { type: 'json' }; import { quz } from './quz.js' with { attr1: 'quz' }; + import * as QuzDuplicate from './quz.js' with { attr1: 'quz' }; import { quz as quz2 } from './quz.js' with { attr2: 'quark', attr3: 'baz' }; + import * as Quz2Duplicate from './quz.js' with { attr2: 'quark', attr3: 'baz' }; export { foo, bar, quz, quz2 }; `); diff --git a/test/parallel/test-vm-module-synthetic.js b/test/parallel/test-vm-module-synthetic.js index 831387ddbd2a26..c32a99ede12cfa 100644 --- a/test/parallel/test-vm-module-synthetic.js +++ b/test/parallel/test-vm-module-synthetic.js @@ -58,12 +58,10 @@ const assert = require('assert'); } { - const s = new SyntheticModule([], () => {}); - assert.throws(() => { - s.setExport('name', 'value'); - }, { - code: 'ERR_VM_MODULE_STATUS', - }); + const s = new SyntheticModule(['name'], () => {}); + // Exports of SyntheticModule can be immediately set after creation. + // No link is required. + s.setExport('name', 'value'); } for (const value of [null, {}, SyntheticModule.prototype]) {