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`.
PR-URL: #59527
Backport-PR-URL: #60152Fixes: #59480
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
Reviewed-By: Marco Ippolito <marcoippolito54@gmail.com>

Author: legendecas

src/module_wrap.cc

@@ -575,23 +575,13 @@ void ModuleWrap::GetModuleRequests(const FunctionCallbackInfo<Value>& args) {
 
 // moduleWrap.link(moduleWraps)
 voidModuleWrap::Link(const FunctionCallbackInfo<Value>& args) {
- Realm* realm = Realm::GetCurrent(args);
  Isolate* isolate = args.GetIsolate();
 
  ModuleWrap* dependent;
 ASSIGN_OR_RETURN_UNWRAP(&dependent, args.This());
 
 CHECK_EQ(args.Length(), 1);
 
- Local<Data> linked_requests =
- args.This()->GetInternalField(kLinkedRequestsSlot);
-if (linked_requests->IsValue() &&
- !linked_requests.As<Value>()->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<FixedArray> requests =
  dependent->module_.Get(isolate)->GetModuleRequests();
  Local<Array> modules = args[0].As<Array>();

test/parallel/test-vm-module-link-shared-deps.js

@@ -0,0 +1,82 @@
+// Flags: --experimental-vm-modules
+
+'use strict';
+
+constcommon=require('../common');
+constvm=require('vm');
+constassert=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
+
+constsources={
+'./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",
+ };
+ `,
+};
+
+constmoduleCache=newMap();
+
+functiongetModuleInstance(identifier){
+letmodule=moduleCache.get(identifier);
+
+if(!module){
+module=newvm.SourceTextModule(sources[identifier],{
+ identifier,
+});
+moduleCache.set(identifier,module);
+}
+
+returnmodule;
+}
+
+asyncfunctionesmImport(identifier){
+constmodule=getModuleInstance(identifier);
+constrequests=[];
+
+awaitmodule.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.
+returnPromise.resolve(getModuleInstance(specifier));
+});
+
+awaitmodule.evaluate();
+return[module.namespace,requests];
+}
+
+asyncfunctiontest(){
+const{0: mod,1: requests}=awaitesmImport('./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());