Skip to content

Commit

Permalink
src, lib: return promises from link
Browse files Browse the repository at this point in the history
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>
  • Loading branch information
devsnek authored and MylesBorins committed Feb 21, 2018
1 parent 46b8481 commit 48220ed
Show file tree
Hide file tree
Showing 2 changed files with 17 additions and 15 deletions.
24 changes: 10 additions & 14 deletions lib/internal/loader/ModuleJob.js
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -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.
Expand Down
8 changes: 7 additions & 1 deletion src/module_wrap.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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);
Expand All @@ -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) {
Expand Down

0 comments on commit 48220ed

Please sign in to comment.