Skip to content

Commit b4056ce

Browse files
legendecaslegendecas
committed
lib,src: expose cached ModuleWrap.hasAsyncGraph
`v8::Module::IsGraphAsync()` traverses the dependencies to find if they contain TLA each time. `ModuleWrap.hasAsyncGraph()` caches the result and exposes the property to JS land so that the presence of the property `module.hasAsyncGraph` can be consistent. This also merges the `intantiateSync`/`instantiate` and `getNamespaceSync`/`getNamespace` as they are always sync.
1 parent fb22c7f commit b4056ce

File tree

7 files changed

+53
-97
lines changed

7 files changed

+53
-97
lines changed

‎lib/internal/bootstrap/realm.js‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -359,7 +359,6 @@ class BuiltinModule {
359359
this.setExport('default', builtin.exports);
360360
});
361361
// Ensure immediate sync execution to capture exports now
362-
this.module.link([]);
363362
this.module.instantiate();
364363
this.module.evaluate(-1, false);
365364
return this.module;

‎lib/internal/modules/esm/loader.js‎

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -387,13 +387,16 @@ class ModuleLoader {
387387
if (!job.module) {
388388
assert.fail(getRaceMessage(filename, parentFilename));
389389
}
390-
if (job.module.hasAsyncGraph) {
390+
if (job.module.hasAsyncGraph()) {
391391
throw new ERR_REQUIRE_ASYNC_MODULE(filename, parentFilename);
392392
}
393393
const status = job.module.getStatus();
394394
debug('Module status', job, status);
395395
if (status === kEvaluated) {
396-
return { wrap: job.module, namespace: job.module.getNamespaceSync(filename, parentFilename) };
396+
if (job.module.hasAsyncGraph()) {
397+
throw new ERR_REQUIRE_ASYNC_MODULE(filename, parentFilename);
398+
}
399+
return { wrap: job.module, namespace: job.module.getNamespace() };
397400
} else if (status === kInstantiated) {
398401
// When it's an async job cached by another import request,
399402
// which has finished linking but has not started its

‎lib/internal/modules/esm/module_job.js‎

Lines changed: 12 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -323,19 +323,18 @@ class ModuleJob extends ModuleJobBase {
323323
assert(this.module instanceof ModuleWrap);
324324
let status = this.module.getStatus();
325325

326+
const filename = urlToFilename(this.url);
327+
const parentFilename = urlToFilename(parent?.filename);
328+
326329
debug('ModuleJob.runSync()', status, this.module);
327330
// FIXME(joyeecheung): this cannot fully handle < kInstantiated. Make the linking
328331
// fully synchronous instead.
329332
if (status === kUninstantiated) {
330-
this.module.hasAsyncGraph = this.module.instantiateSync();
333+
this.module.instantiate();
331334
status = this.module.getStatus();
332335
}
333336
if (status === kInstantiated || status === kErrored) {
334-
const filename = urlToFilename(this.url);
335-
const parentFilename = urlToFilename(parent?.filename);
336-
this.module.hasAsyncGraph ??= this.module.isGraphAsync();
337-
338-
if (this.module.async && !getOptionValue('--experimental-print-required-tla')) {
337+
if (this.module.hasAsyncGraph() && !getOptionValue('--experimental-print-required-tla')) {
339338
throw new ERR_REQUIRE_ASYNC_MODULE(filename, parentFilename);
340339
}
341340
if (status === kInstantiated) {
@@ -345,14 +344,17 @@ class ModuleJob extends ModuleJobBase {
345344
}
346345
throw this.module.getError();
347346
} else if (status === kEvaluating || status === kEvaluated) {
347+
if (this.module.hasAsyncGraph()) {
348+
throw new ERR_REQUIRE_ASYNC_MODULE(filename, parentFilename);
349+
}
348350
// kEvaluating can show up when this is being used to deal with CJS <-> CJS cycles.
349351
// Allow it for now, since we only need to ban ESM <-> CJS cycles which would be
350352
// detected earlier during the linking phase, though the CJS handling in the ESM
351353
// loader won't be able to emit warnings on pending circular exports like what
352354
// the CJS loader does.
353355
// TODO(joyeecheung): remove the re-invented require() in the ESM loader and
354356
// always handle CJS using the CJS loader to eliminate the quirks.
355-
return { __proto__: null, module: this.module, namespace: this.module.getNamespaceSync() };
357+
return { __proto__: null, module: this.module, namespace: this.module.getNamespace() };
356358
}
357359
assert.fail(`Unexpected module status ${status}.`);
358360
}
@@ -472,7 +474,7 @@ class ModuleJobSync extends ModuleJobBase {
472474
}
473475
return { __proto__: null, module: this.module };
474476
} else if (status === kInstantiated) {
475-
// The evaluation may have been canceled because instantiateSync() detected TLA first.
477+
// The evaluation may have been canceled because instantiate() detected TLA first.
476478
// But when it is imported again, it's fine to re-evaluate it asynchronously.
477479
const timeout = -1;
478480
const breakOnSigint = false;
@@ -490,7 +492,7 @@ class ModuleJobSync extends ModuleJobBase {
490492
debug('ModuleJobSync.runSync()', this.module);
491493
assert(this.phase === kEvaluationPhase);
492494
// TODO(joyeecheung): add the error decoration logic from the async instantiate.
493-
this.module.hasAsyncGraph = this.module.instantiateSync();
495+
this.module.instantiate();
494496
// If --experimental-print-required-tla is true, proceeds to evaluation even
495497
// if it's async because we want to search for the TLA and help users locate
496498
// them.
@@ -500,7 +502,7 @@ class ModuleJobSync extends ModuleJobBase {
500502
// it runs into cached asynchronous modules that are not yet fetched.
501503
const parentFilename = urlToFilename(parent?.filename);
502504
const filename = urlToFilename(this.url);
503-
if (this.module.hasAsyncGraph && !getOptionValue('--experimental-print-required-tla')) {
505+
if (this.module.hasAsyncGraph() && !getOptionValue('--experimental-print-required-tla')) {
504506
throw new ERR_REQUIRE_ASYNC_MODULE(filename, parentFilename);
505507
}
506508
setHasStartedUserESMExecution();

‎lib/internal/vm/module.js‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -469,7 +469,6 @@ class SyntheticModule extends Module {
469469
identifier,
470470
});
471471
// A synthetic module does not have dependencies.
472-
this[kWrap].link([]);
473472
this[kWrap].instantiate();
474473
}
475474

‎src/module_wrap.cc‎

Lines changed: 31 additions & 80 deletions
Original file line numberDiff line numberDiff line change
@@ -158,6 +158,8 @@ ModuleWrap::ModuleWrap(Realm* realm,
158158

159159
if (!synthetic_evaluation_step->IsUndefined()) {
160160
synthetic_ = true;
161+
// Synthetic modules have no dependencies.
162+
linked_ = true;
161163
}
162164
MakeWeak();
163165
module_.SetWeak();
@@ -240,7 +242,7 @@ Maybe<bool> ModuleWrap::CheckUnsettledTopLevelAwait() {
240242
return Just(true);
241243
}
242244

243-
if (!module->IsGraphAsync()) { // There is no TLA, no need to check.
245+
if (!HasAsyncGraph()) { // There is no TLA, no need to check.
244246
return Just(true);
245247
}
246248

@@ -263,6 +265,16 @@ Maybe<bool> ModuleWrap::CheckUnsettledTopLevelAwait() {
263265
return Just(false);
264266
}
265267

268+
bool ModuleWrap::HasAsyncGraph() {
269+
if (!has_async_graph_.has_value()) {
270+
Isolate* isolate = env()->isolate();
271+
HandleScope scope(isolate);
272+
273+
has_async_graph_ = module_.Get(isolate)->IsGraphAsync();
274+
}
275+
return has_async_graph_.value();
276+
}
277+
266278
Local<PrimitiveArray> ModuleWrap::GetHostDefinedOptions(
267279
Isolate* isolate, Local<Symbol> id_symbol) {
268280
Local<PrimitiveArray> host_defined_options =
@@ -673,35 +685,6 @@ void ModuleWrap::Link(const FunctionCallbackInfo<Value>& args) {
673685
dependent->linked_ = true;
674686
}
675687

676-
void ModuleWrap::Instantiate(const FunctionCallbackInfo<Value>& args) {
677-
Realm* realm = Realm::GetCurrent(args);
678-
Isolate* isolate = args.GetIsolate();
679-
ModuleWrap* obj;
680-
ASSIGN_OR_RETURN_UNWRAP(&obj, args.This());
681-
Local<Context> context = obj->context();
682-
Local<Module> module = obj->module_.Get(isolate);
683-
684-
if (!obj->IsLinked()) {
685-
THROW_ERR_VM_MODULE_LINK_FAILURE(realm->env(), "module is not linked");
686-
return;
687-
}
688-
689-
TryCatchScope try_catch(realm->env());
690-
USE(module->InstantiateModule(
691-
context, ResolveModuleCallback, ResolveSourceCallback));
692-
693-
if (try_catch.HasCaught() && !try_catch.HasTerminated()) {
694-
CHECK(!try_catch.Message().IsEmpty());
695-
CHECK(!try_catch.Exception().IsEmpty());
696-
AppendExceptionLine(realm->env(),
697-
try_catch.Exception(),
698-
try_catch.Message(),
699-
ErrorHandlingMode::MODULE_ERROR);
700-
try_catch.ReThrow();
701-
return;
702-
}
703-
}
704-
705688
void ModuleWrap::Evaluate(const FunctionCallbackInfo<Value>& args) {
706689
Realm* realm = Realm::GetCurrent(args);
707690
Isolate* isolate = realm->isolate();
@@ -783,7 +766,7 @@ void ModuleWrap::Evaluate(const FunctionCallbackInfo<Value>& args) {
783766
}
784767
}
785768

786-
void ModuleWrap::InstantiateSync(const FunctionCallbackInfo<Value>& args) {
769+
void ModuleWrap::Instantiate(const FunctionCallbackInfo<Value>& args) {
787770
Realm* realm = Realm::GetCurrent(args);
788771
Isolate* isolate = args.GetIsolate();
789772
ModuleWrap* obj;
@@ -792,6 +775,11 @@ void ModuleWrap::InstantiateSync(const FunctionCallbackInfo<Value>& args) {
792775
Local<Module> module = obj->module_.Get(isolate);
793776
Environment* env = realm->env();
794777

778+
if (!obj->IsLinked()) {
779+
THROW_ERR_VM_MODULE_LINK_FAILURE(env, "module is not linked");
780+
return;
781+
}
782+
795783
{
796784
TryCatchScope try_catch(env);
797785
USE(module->InstantiateModule(
@@ -811,7 +799,7 @@ void ModuleWrap::InstantiateSync(const FunctionCallbackInfo<Value>& args) {
811799

812800
// TODO(joyeecheung): record Module::HasTopLevelAwait() in every ModuleWrap
813801
// and infer the asynchronicity from a module's children during linking.
814-
args.GetReturnValue().Set(module->IsGraphAsync());
802+
args.GetReturnValue().Set(obj->HasAsyncGraph());
815803
}
816804

817805
Maybe<void> ThrowIfPromiseRejected(Realm* realm, Local<Promise> promise) {
@@ -879,7 +867,7 @@ void ModuleWrap::EvaluateSync(const FunctionCallbackInfo<Value>& args) {
879867
return;
880868
}
881869

882-
if (module->IsGraphAsync()) {
870+
if (obj->HasAsyncGraph()) {
883871
CHECK(env->options()->print_required_tla);
884872
auto stalled_messages =
885873
std::get<1>(module->GetStalledTopLevelAwaitMessages(isolate));
@@ -901,52 +889,15 @@ void ModuleWrap::EvaluateSync(const FunctionCallbackInfo<Value>& args) {
901889
args.GetReturnValue().Set(module->GetModuleNamespace());
902890
}
903891

904-
void ModuleWrap::GetNamespaceSync(const FunctionCallbackInfo<Value>& args) {
905-
Realm* realm = Realm::GetCurrent(args);
906-
Isolate* isolate = args.GetIsolate();
907-
ModuleWrap* obj;
908-
ASSIGN_OR_RETURN_UNWRAP(&obj, args.This());
909-
Local<Module> module = obj->module_.Get(isolate);
910-
911-
switch (module->GetStatus()) {
912-
case Module::Status::kUninstantiated:
913-
case Module::Status::kInstantiating:
914-
return realm->env()->ThrowError(
915-
"Cannot get namespace, module has not been instantiated");
916-
case Module::Status::kInstantiated:
917-
case Module::Status::kEvaluating:
918-
case Module::Status::kEvaluated:
919-
case Module::Status::kErrored:
920-
break;
921-
}
922-
923-
if (module->IsGraphAsync()) {
924-
return THROW_ERR_REQUIRE_ASYNC_MODULE(realm->env(), args[0], args[1]);
925-
}
926-
Local<Value> result = module->GetModuleNamespace();
927-
args.GetReturnValue().Set(result);
928-
}
929-
930892
void ModuleWrap::GetNamespace(const FunctionCallbackInfo<Value>& args) {
931893
Realm* realm = Realm::GetCurrent(args);
932894
Isolate* isolate = args.GetIsolate();
933895
ModuleWrap* obj;
934896
ASSIGN_OR_RETURN_UNWRAP(&obj, args.This());
935897

936898
Local<Module> module = obj->module_.Get(isolate);
937-
938-
switch (module->GetStatus()) {
939-
case Module::Status::kUninstantiated:
940-
case Module::Status::kInstantiating:
941-
return realm->env()->ThrowError(
942-
"cannot get namespace, module has not been instantiated");
943-
case Module::Status::kInstantiated:
944-
case Module::Status::kEvaluating:
945-
case Module::Status::kEvaluated:
946-
case Module::Status::kErrored:
947-
break;
948-
default:
949-
UNREACHABLE();
899+
if (module->GetStatus() < Module::kInstantiated) {
900+
return THROW_ERR_MODULE_NOT_INSTANTIATED(realm->env());
950901
}
951902

952903
Local<Value> result = module->GetModuleNamespace();
@@ -997,14 +948,18 @@ void ModuleWrap::GetStatus(const FunctionCallbackInfo<Value>& args) {
997948
args.GetReturnValue().Set(module->GetStatus());
998949
}
999950

1000-
void ModuleWrap::IsGraphAsync(const FunctionCallbackInfo<Value>& args) {
951+
void ModuleWrap::HasAsyncGraph(const FunctionCallbackInfo<Value>& args) {
1001952
Isolate* isolate = args.GetIsolate();
953+
Environment* env = Environment::GetCurrent(isolate);
1002954
ModuleWrap* obj;
1003955
ASSIGN_OR_RETURN_UNWRAP(&obj, args.This());
1004956

1005957
Local<Module> module = obj->module_.Get(isolate);
958+
if (module->GetStatus() < Module::kInstantiated) {
959+
return THROW_ERR_MODULE_NOT_INSTANTIATED(env);
960+
}
1006961

1007-
args.GetReturnValue().Set(module->IsGraphAsync());
962+
args.GetReturnValue().Set(obj->HasAsyncGraph());
1008963
}
1009964

1010965
void ModuleWrap::GetError(const FunctionCallbackInfo<Value>& args) {
@@ -1417,9 +1372,7 @@ void ModuleWrap::CreatePerIsolateProperties(IsolateData* isolate_data,
14171372

14181373
SetProtoMethod(isolate, tpl, "link", Link);
14191374
SetProtoMethod(isolate, tpl, "getModuleRequests", GetModuleRequests);
1420-
SetProtoMethod(isolate, tpl, "instantiateSync", InstantiateSync);
14211375
SetProtoMethod(isolate, tpl, "evaluateSync", EvaluateSync);
1422-
SetProtoMethod(isolate, tpl, "getNamespaceSync", GetNamespaceSync);
14231376
SetProtoMethod(isolate, tpl, "instantiate", Instantiate);
14241377
SetProtoMethod(isolate, tpl, "evaluate", Evaluate);
14251378
SetProtoMethod(isolate, tpl, "setExport", SetSyntheticExport);
@@ -1429,7 +1382,7 @@ void ModuleWrap::CreatePerIsolateProperties(IsolateData* isolate_data,
14291382
isolate, tpl, "createCachedData", CreateCachedData);
14301383
SetProtoMethodNoSideEffect(isolate, tpl, "getNamespace", GetNamespace);
14311384
SetProtoMethodNoSideEffect(isolate, tpl, "getStatus", GetStatus);
1432-
SetProtoMethodNoSideEffect(isolate, tpl, "isGraphAsync", IsGraphAsync);
1385+
SetProtoMethodNoSideEffect(isolate, tpl, "hasAsyncGraph", HasAsyncGraph);
14331386
SetProtoMethodNoSideEffect(isolate, tpl, "getError", GetError);
14341387
SetConstructorFunction(isolate, target, "ModuleWrap", tpl);
14351388
isolate_data->set_module_wrap_constructor_template(tpl);
@@ -1479,9 +1432,7 @@ void ModuleWrap::RegisterExternalReferences(
14791432

14801433
registry->Register(Link);
14811434
registry->Register(GetModuleRequests);
1482-
registry->Register(InstantiateSync);
14831435
registry->Register(EvaluateSync);
1484-
registry->Register(GetNamespaceSync);
14851436
registry->Register(Instantiate);
14861437
registry->Register(Evaluate);
14871438
registry->Register(SetSyntheticExport);
@@ -1491,7 +1442,7 @@ void ModuleWrap::RegisterExternalReferences(
14911442
registry->Register(GetNamespace);
14921443
registry->Register(GetStatus);
14931444
registry->Register(GetError);
1494-
registry->Register(IsGraphAsync);
1445+
registry->Register(HasAsyncGraph);
14951446

14961447
registry->Register(CreateRequiredModuleFacade);
14971448

‎src/module_wrap.h‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -121,6 +121,7 @@ class ModuleWrap : public BaseObject {
121121

122122
v8::Local<v8::Context> context() const;
123123
v8::Maybe<bool> CheckUnsettledTopLevelAwait();
124+
bool HasAsyncGraph();
124125

125126
SET_MEMORY_INFO_NAME(ModuleWrap)
126127
SET_SELF_SIZE(ModuleWrap)
@@ -168,9 +169,7 @@ class ModuleWrap : public BaseObject {
168169
static void New(const v8::FunctionCallbackInfo<v8::Value>& args);
169170
static void GetModuleRequests(
170171
const v8::FunctionCallbackInfo<v8::Value>& args);
171-
static void InstantiateSync(const v8::FunctionCallbackInfo<v8::Value>& args);
172172
static void EvaluateSync(const v8::FunctionCallbackInfo<v8::Value>& args);
173-
static void GetNamespaceSync(const v8::FunctionCallbackInfo<v8::Value>& args);
174173
static void SetModuleSourceObject(
175174
const v8::FunctionCallbackInfo<v8::Value>& args);
176175
static void GetModuleSourceObject(
@@ -180,7 +179,7 @@ class ModuleWrap : public BaseObject {
180179
static void Instantiate(const v8::FunctionCallbackInfo<v8::Value>& args);
181180
static void Evaluate(const v8::FunctionCallbackInfo<v8::Value>& args);
182181
static void GetNamespace(const v8::FunctionCallbackInfo<v8::Value>& args);
183-
static void IsGraphAsync(const v8::FunctionCallbackInfo<v8::Value>& args);
182+
static void HasAsyncGraph(const v8::FunctionCallbackInfo<v8::Value>& args);
184183
static void GetStatus(const v8::FunctionCallbackInfo<v8::Value>& args);
185184
static void GetError(const v8::FunctionCallbackInfo<v8::Value>& args);
186185

@@ -219,6 +218,7 @@ class ModuleWrap : public BaseObject {
219218
contextify::ContextifyContext* contextify_context_ = nullptr;
220219
bool synthetic_ = false;
221220
bool linked_ = false;
221+
std::optional<bool> has_async_graph_ = std::nullopt;
222222
int module_hash_;
223223
};
224224

‎src/node_errors.h‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -111,6 +111,7 @@ void OOMErrorHandler(const char* location, const v8::OOMDetails& details);
111111
V(ERR_MISSING_PASSPHRASE, TypeError) \
112112
V(ERR_MISSING_PLATFORM_FOR_WORKER, Error) \
113113
V(ERR_MODULE_NOT_FOUND, Error) \
114+
V(ERR_MODULE_NOT_INSTANTIATED, Error) \
114115
V(ERR_MODULE_LINK_MISMATCH, TypeError) \
115116
V(ERR_NON_CONTEXT_AWARE_DISABLED, Error) \
116117
V(ERR_OPERATION_FAILED, TypeError) \
@@ -230,6 +231,7 @@ ERRORS_WITH_CODE(V)
230231
V(ERR_MISSING_PLATFORM_FOR_WORKER, \
231232
"The V8 platform used by this instance of Node does not support " \
232233
"creating Workers") \
234+
V(ERR_MODULE_NOT_INSTANTIATED, "Module is not instantiated") \
233235
V(ERR_NON_CONTEXT_AWARE_DISABLED, \
234236
"Loading non context-aware native addons has been disabled") \
235237
V(ERR_SCRIPT_EXECUTION_INTERRUPTED, \

0 commit comments

Comments
 (0)