From 24d8ec68e6e4aa3df971bedf2c397436d9ac8297 Mon Sep 17 00:00:00 2001
From: Gus Caplan <me@gus.host>
Date: Fri, 26 Jan 2018 09:38:31 -0600
Subject: [PATCH] src, lib: return promises from link

Returns the promises created by link so that they can be awaited to get
rid of race conditions while resolving and loading modules.

PR-URL: https://github.com/nodejs/node/pull/18394
Reviewed-By: Tiancheng "Timothy" Gu <timothygu99@gmail.com>
---
 lib/internal/loader/ModuleJob.js | 24 ++++++++++--------------
 src/module_wrap.cc               |  8 +++++++-
 2 files changed, 17 insertions(+), 15 deletions(-)

diff --git a/lib/internal/loader/ModuleJob.js b/lib/internal/loader/ModuleJob.js
index 2d6325b85c6322..db37765b20bd0c 100644
--- a/lib/internal/loader/ModuleJob.js
+++ b/lib/internal/loader/ModuleJob.js
@@ -6,9 +6,6 @@ const { decorateErrorStack } = require('internal/util');
 const assert = require('assert');
 const resolvedPromise = SafePromise.resolve();
 
-const enableDebug = (process.env.NODE_DEBUG || '').match(/\besm\b/) ||
-    process.features.debug;
-
 /* A ModuleJob tracks the loading of a single Module, and the ModuleJobs of
  * its dependencies, over time. */
 class ModuleJob {
@@ -27,7 +24,6 @@ class ModuleJob {
 
     // Wait for the ModuleWrap instance being linked with all dependencies.
     const link = async () => {
-      const dependencyJobs = [];
       ({ module: this.module,
          reflect: this.reflect } = await this.modulePromise);
       if (inspectBrk) {
@@ -35,17 +31,17 @@ class ModuleJob {
         initWrapper(this.module.instantiate, this.module);
       }
       assert(this.module instanceof ModuleWrap);
-      this.module.link(async (dependencySpecifier) => {
-        const dependencyJobPromise =
-            this.loader.getModuleJob(dependencySpecifier, url);
-        dependencyJobs.push(dependencyJobPromise);
-        const dependencyJob = await dependencyJobPromise;
-        return (await dependencyJob.modulePromise).module;
+
+      const dependencyJobs = [];
+      const promises = this.module.link(async (specifier) => {
+        const jobPromise = this.loader.getModuleJob(specifier, url);
+        dependencyJobs.push(jobPromise);
+        return (await (await jobPromise).modulePromise).module;
       });
-      if (enableDebug) {
-        // Make sure all dependencies are entered into the list synchronously.
-        Object.freeze(dependencyJobs);
-      }
+
+      if (promises !== undefined)
+        await SafePromise.all(promises);
+
       return SafePromise.all(dependencyJobs);
     };
     // Promise for the list of all dependencyJobs.
diff --git a/src/module_wrap.cc b/src/module_wrap.cc
index b8970d4fb3222f..eddc0d6d6ebd3d 100644
--- a/src/module_wrap.cc
+++ b/src/module_wrap.cc
@@ -13,6 +13,7 @@ namespace loader {
 
 using node::url::URL;
 using node::url::URL_FLAGS_FAILED;
+using v8::Array;
 using v8::Context;
 using v8::Function;
 using v8::FunctionCallbackInfo;
@@ -151,6 +152,9 @@ void ModuleWrap::Link(const FunctionCallbackInfo<Value>& args) {
   obj->linked_ = true;
   Local<Module> module(obj->module_.Get(isolate));
 
+  Local<Array> promises = Array::New(isolate,
+                                     module->GetModuleRequestsLength());
+
   // call the dependency resolve callbacks
   for (int i = 0; i < module->GetModuleRequestsLength(); i++) {
     Local<String> specifier = module->GetModuleRequest(i);
@@ -173,9 +177,11 @@ void ModuleWrap::Link(const FunctionCallbackInfo<Value>& args) {
     }
     Local<Promise> resolve_promise = resolve_return_value.As<Promise>();
     obj->resolve_cache_[specifier_std].Reset(env->isolate(), resolve_promise);
+
+    promises->Set(mod_context, specifier, resolve_promise).FromJust();
   }
 
-  args.GetReturnValue().Set(that);
+  args.GetReturnValue().Set(promises);
 }
 
 void ModuleWrap::Instantiate(const FunctionCallbackInfo<Value>& args) {