From 8b1f4eb7e689527cd976155122e3ed9d81a078d1 Mon Sep 17 00:00:00 2001 From: Chengzhong Wu Date: Tue, 19 Aug 2025 00:42:35 +0100 Subject: [PATCH] module: allow overriding linked requests for a ModuleWrap This allows overriding linked requests for a `ModuleWrap`. The `statusOverride` in `vm.SourceTextModule` could call `moduleWrap.link` a second time when `statusOverride` of `linking` is set to undefined. Overriding of linked requests should be no harm but better to be avoided. However, this will require a follow-up fix on `statusOverride` in `vm.SourceTextModule`. --- src/module_wrap.cc | 10 --- .../test-vm-module-link-shared-deps.js | 82 +++++++++++++++++++ 2 files changed, 82 insertions(+), 10 deletions(-) create mode 100644 test/parallel/test-vm-module-link-shared-deps.js diff --git a/src/module_wrap.cc b/src/module_wrap.cc index 28843f6206e62a..72d910fa4a8144 100644 --- a/src/module_wrap.cc +++ b/src/module_wrap.cc @@ -604,7 +604,6 @@ void ModuleWrap::GetModuleRequests(const FunctionCallbackInfo& args) { // moduleWrap.link(moduleWraps) void ModuleWrap::Link(const FunctionCallbackInfo& args) { - Realm* realm = Realm::GetCurrent(args); Isolate* isolate = args.GetIsolate(); ModuleWrap* dependent; @@ -612,15 +611,6 @@ void ModuleWrap::Link(const FunctionCallbackInfo& args) { 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(); 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());