From a348609d4d2d3b29e97c2d894cae353e8c1da6bb Mon Sep 17 00:00:00 2001
From: Gus Caplan <me@gus.host>
Date: Wed, 3 Mar 2021 21:15:10 -0600
Subject: [PATCH] vm: fix link with invalid graph

This fixes an annoying bug where the entry module would link before its
dependencies were linked. This is fixed by forcing modules to wait for
all dependency linker promises to resolve.

Fixes: https://github.com/nodejs/node/issues/37426

Signed-off-by: snek <me@gus.host>
---
 lib/internal/vm/module.js                  | 43 +++++++++++++---------
 test/parallel/test-vm-module-link-37426.js | 23 ++++++++++++
 2 files changed, 48 insertions(+), 18 deletions(-)
 create mode 100644 test/parallel/test-vm-module-link-37426.js

diff --git a/lib/internal/vm/module.js b/lib/internal/vm/module.js
index 56164b0d8b98d4..ccdfd2ec2fb612 100644
--- a/lib/internal/vm/module.js
+++ b/lib/internal/vm/module.js
@@ -255,6 +255,7 @@ const kNoError = Symbol('kNoError');
 class SourceTextModule extends Module {
   #error = kNoError;
   #statusOverride;
+  #waitingLinkPromises;
 
   constructor(sourceText, options = {}) {
     validateString(sourceText, 'sourceText');
@@ -308,31 +309,37 @@ class SourceTextModule extends Module {
     this[kLink] = async (linker) => {
       this.#statusOverride = 'linking';
 
-      const promises = this[kWrap].link(async (identifier) => {
-        const module = await linker(identifier, this);
-        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_LINKING_ERRORED();
-        }
-        if (module.status === 'unlinked') {
-          await module[kLink](linker);
-        }
-        return module[kWrap];
-      });
-
       try {
+        const promises = this[kWrap].link(async (identifier) => {
+          const module = await linker(identifier, this);
+          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_LINKING_ERRORED();
+          }
+          if (module.status === 'unlinked') {
+            await module[kLink](linker);
+          }
+          if (module.status === 'linking') {
+            await module.#waitingLinkPromises;
+          }
+          return module[kWrap];
+        });
+
         if (promises !== undefined) {
-          await PromiseAll(promises);
+          assert(this.#waitingLinkPromises === undefined);
+          this.#waitingLinkPromises = PromiseAll(promises);
         }
+        await this.#waitingLinkPromises;
       } catch (e) {
         this.#error = e;
         throw e;
       } finally {
+        this.#waitingLinkPromises = undefined;
         this.#statusOverride = undefined;
       }
     };
diff --git a/test/parallel/test-vm-module-link-37426.js b/test/parallel/test-vm-module-link-37426.js
new file mode 100644
index 00000000000000..8eece9c1446607
--- /dev/null
+++ b/test/parallel/test-vm-module-link-37426.js
@@ -0,0 +1,23 @@
+// Flags: --experimental-vm-modules
+
+'use strict';
+
+const common = require('../common');
+const { SourceTextModule, SyntheticModule } = require('vm');
+
+const tm = new SourceTextModule('import "a"');
+
+tm
+  .link(async () => {
+    const tm2 = new SourceTextModule('import "b"');
+    tm2.link(async () => {
+      const sm = new SyntheticModule([], () => {});
+      await sm.link(() => {});
+      await sm.evaluate();
+      return sm;
+    }).then(() => tm2.evaluate());
+    return tm2;
+  })
+  .then(() => tm.evaluate())
+  .then(common.mustCall())
+  .catch(common.mustNotCall());