Commit 64c36d3
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: #18394
Reviewed-By: Tiancheng "Timothy" Gu <timothygu99@gmail.com>1 parent e00bb16 commit 64c36d3
2 files changed
Lines changed: 17 additions & 15 deletions
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
@@ -6,9 +6,6 @@ const { decorateErrorStack } = require('internal/util'); | |||
| 6 | 6 | const assert = require('assert'); | |
| 7 | 7 | const resolvedPromise = SafePromise.resolve(); | |
| 8 | 8 | ||
| 9 | - const enableDebug = (process.env.NODE_DEBUG || '').match(/\besm\b/) || | ||
| 10 | - process.features.debug; | ||
| 11 | - | ||
| 12 | 9 | /* A ModuleJob tracks the loading of a single Module, and the ModuleJobs of | |
| 13 | 10 | * its dependencies, over time. */ | |
| 14 | 11 | class ModuleJob { | |
@@ -27,25 +24,24 @@ class ModuleJob { | |||
| 27 | 24 | ||
| 28 | 25 | // Wait for the ModuleWrap instance being linked with all dependencies. | |
| 29 | 26 | const link = async () => { | |
| 30 | - const dependencyJobs = []; | ||
| 31 | 27 | ({ module: this.module, | |
| 32 | 28 | reflect: this.reflect } = await this.modulePromise); | |
| 33 | 29 | if (inspectBrk) { | |
| 34 | 30 | const initWrapper = process.binding('inspector').callAndPauseOnStart; | |
| 35 | 31 | initWrapper(this.module.instantiate, this.module); | |
| 36 | 32 | } | |
| 37 | 33 | assert(this.module instanceof ModuleWrap); | |
| 38 | - this.module.link(async (dependencySpecifier) => { | ||
| 39 | - const dependencyJobPromise = | ||
| 40 | - this.loader.getModuleJob(dependencySpecifier, url); | ||
| 41 | - dependencyJobs.push(dependencyJobPromise); | ||
| 42 | - const dependencyJob = await dependencyJobPromise; | ||
| 43 | - return (await dependencyJob.modulePromise).module; | ||
| 34 | + | ||
| 35 | + const dependencyJobs = []; | ||
| 36 | + const promises = this.module.link(async (specifier) => { | ||
| 37 | + const jobPromise = this.loader.getModuleJob(specifier, url); | ||
| 38 | + dependencyJobs.push(jobPromise); | ||
| 39 | + return (await (await jobPromise).modulePromise).module; | ||
| 44 | 40 | }); | |
| 45 | - if (enableDebug) { | ||
| 46 | - // Make sure all dependencies are entered into the list synchronously. | ||
| 47 | - Object.freeze(dependencyJobs); | ||
| 48 | - } | ||
| 41 | + | ||
| 42 | + if (promises !== undefined) | ||
| 43 | + await SafePromise.all(promises); | ||
| 44 | + | ||
| 49 | 45 | return SafePromise.all(dependencyJobs); | |
| 50 | 46 | }; | |
| 51 | 47 | // Promise for the list of all dependencyJobs. | |
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
@@ -13,6 +13,7 @@ namespace loader { | |||
| 13 | 13 | ||
| 14 | 14 | using node::url::URL; | |
| 15 | 15 | using node::url::URL_FLAGS_FAILED; | |
| 16 | + using v8::Array; | ||
| 16 | 17 | using v8::Context; | |
| 17 | 18 | using v8::Function; | |
| 18 | 19 | using v8::FunctionCallbackInfo; | |
@@ -151,6 +152,9 @@ void ModuleWrap::Link(const FunctionCallbackInfo<Value>& args) { | |||
| 151 | 152 | obj->linked_ = true; | |
| 152 | 153 | Local<Module> module(obj->module_.Get(isolate)); | |
| 153 | 154 | ||
| 155 | + Local<Array> promises = Array::New(isolate, | ||
| 156 | + module->GetModuleRequestsLength()); | ||
| 157 | + | ||
| 154 | 158 | // call the dependency resolve callbacks | |
| 155 | 159 | for (int i = 0; i < module->GetModuleRequestsLength(); i++) { | |
| 156 | 160 | Local<String> specifier = module->GetModuleRequest(i); | |
@@ -173,9 +177,11 @@ void ModuleWrap::Link(const FunctionCallbackInfo<Value>& args) { | |||
| 173 | 177 | } | |
| 174 | 178 | Local<Promise> resolve_promise = resolve_return_value.As<Promise>(); | |
| 175 | 179 | obj->resolve_cache_[specifier_std].Reset(env->isolate(), resolve_promise); | |
| 180 | + | ||
| 181 | + promises->Set(mod_context, specifier, resolve_promise).FromJust(); | ||
| 176 | 182 | } | |
| 177 | 183 | ||
| 178 | - args.GetReturnValue().Set(that); | ||
| 184 | + args.GetReturnValue().Set(promises); | ||
| 179 | 185 | } | |
| 180 | 186 | ||
| 181 | 187 | void ModuleWrap::Instantiate(const FunctionCallbackInfo<Value>& args) { | |
0 commit comments