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: #60152
Fixes: #59480
Reviewed-By: Joyee Cheung <[email protected]>
Reviewed-By: Marco Ippolito <[email protected]>

Author: legendecas

src/module_wrap.cc

@@ -575,23 +575,13 @@ void ModuleWrap::GetModuleRequests(const FunctionCallbackInfo<Value>& args) {
 
 // moduleWrap.link(moduleWraps)
 void ModuleWrap::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';
+
+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());