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: nodejs#18394 Reviewed-By: Tiancheng "Timothy" Gu <timothygu99@gmail.com>
BridgeAR · Feb 1, 2018 · edbcf7c · edbcf7c
1 parent 3cd7977
commit edbcf7c
Show file tree

Hide file tree

Showing 2 changed files with 16 additions and 15 deletions.
diff --git 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,25 +24,24 @@ 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) {
         const initWrapper = process.binding('inspector').callAndPauseOnStart;
         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
@@ -171,6 +171,9 @@ void ModuleWrap::Link(const FunctionCallbackInfo<Value>& args) {
   Local<Context> mod_context = obj->context_.Get(isolate);
   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);
@@ -193,9 +196,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) {