Skip to content

Commit 64c36d3

Browse files
devsnekMylesBorins
authored andcommitted
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 <[email protected]>
1 parent e00bb16 commit 64c36d3

File tree

2 files changed

+17
-15
lines changed

2 files changed

+17
-15
lines changed

lib/internal/loader/ModuleJob.js

+10-14
Original file line numberDiff line numberDiff line change
@@ -6,9 +6,6 @@ const { decorateErrorStack } = require('internal/util');
66
const assert = require('assert');
77
const resolvedPromise = SafePromise.resolve();
88

9-
const enableDebug = (process.env.NODE_DEBUG || '').match(/\besm\b/) ||
10-
process.features.debug;
11-
129
/* A ModuleJob tracks the loading of a single Module, and the ModuleJobs of
1310
* its dependencies, over time. */
1411
class ModuleJob {
@@ -27,25 +24,24 @@ class ModuleJob {
2724

2825
// Wait for the ModuleWrap instance being linked with all dependencies.
2926
const link = async () => {
30-
const dependencyJobs = [];
3127
({ module: this.module,
3228
reflect: this.reflect } = await this.modulePromise);
3329
if (inspectBrk) {
3430
const initWrapper = process.binding('inspector').callAndPauseOnStart;
3531
initWrapper(this.module.instantiate, this.module);
3632
}
3733
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;
4440
});
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+
4945
return SafePromise.all(dependencyJobs);
5046
};
5147
// Promise for the list of all dependencyJobs.

src/module_wrap.cc

+7-1
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@ namespace loader {
1313

1414
using node::url::URL;
1515
using node::url::URL_FLAGS_FAILED;
16+
using v8::Array;
1617
using v8::Context;
1718
using v8::Function;
1819
using v8::FunctionCallbackInfo;
@@ -151,6 +152,9 @@ void ModuleWrap::Link(const FunctionCallbackInfo<Value>& args) {
151152
obj->linked_ = true;
152153
Local<Module> module(obj->module_.Get(isolate));
153154

155+
Local<Array> promises = Array::New(isolate,
156+
module->GetModuleRequestsLength());
157+
154158
// call the dependency resolve callbacks
155159
for (int i = 0; i < module->GetModuleRequestsLength(); i++) {
156160
Local<String> specifier = module->GetModuleRequest(i);
@@ -173,9 +177,11 @@ void ModuleWrap::Link(const FunctionCallbackInfo<Value>& args) {
173177
}
174178
Local<Promise> resolve_promise = resolve_return_value.As<Promise>();
175179
obj->resolve_cache_[specifier_std].Reset(env->isolate(), resolve_promise);
180+
181+
promises->Set(mod_context, specifier, resolve_promise).FromJust();
176182
}
177183

178-
args.GetReturnValue().Set(that);
184+
args.GetReturnValue().Set(promises);
179185
}
180186

181187
void ModuleWrap::Instantiate(const FunctionCallbackInfo<Value>& args) {

0 commit comments

Comments
 (0)