lib,src: cache 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 allows C++ access of cached `hasAsyncGraph`.

This merges the `intantiateSync`/`instantiate` and
`getNamespaceSync`/`getNamespace` as they are always sync.

Author: legendecas

lib/internal/bootstrap/realm.js

@@ -359,7 +359,6 @@ class BuiltinModule {
         this.setExport('default', builtin.exports);
       });
     // Ensure immediate sync execution to capture exports now
-    this.module.link([]);
     this.module.instantiate();
     this.module.evaluate(-1, false);
     return this.module;

lib/internal/modules/esm/loader.js

@@ -387,13 +387,14 @@ class ModuleLoader {
       if (!job.module) {
         assert.fail(getRaceMessage(filename, parentFilename));
       }
-      if (job.module.hasAsyncGraph) {
-        throw new ERR_REQUIRE_ASYNC_MODULE(filename, parentFilename);
-      }
       const status = job.module.getStatus();
       debug('Module status', job, status);
+      // hasAsyncGraph is available after module been instantiated.
+      if (status >= kInstantiated && job.module.hasAsyncGraph) {
+        throw new ERR_REQUIRE_ASYNC_MODULE(filename, parentFilename);
+      }
       if (status === kEvaluated) {
-        return { wrap: job.module, namespace: job.module.getNamespaceSync(filename, parentFilename) };
+        return { wrap: job.module, namespace: job.module.getNamespace() };
       } else if (status === kInstantiated) {
         // When it's an async job cached by another import request,
         // which has finished linking but has not started its

lib/internal/modules/esm/module_job.js

@@ -327,14 +327,12 @@ class ModuleJob extends ModuleJobBase {
     // FIXME(joyeecheung): this cannot fully handle < kInstantiated. Make the linking
     // fully synchronous instead.
     if (status === kUninstantiated) {
-      this.module.hasAsyncGraph = this.module.instantiateSync();
+      this.module.instantiate();
       status = this.module.getStatus();
     }
     if (status === kInstantiated || status === kErrored) {
       const filename = urlToFilename(this.url);
       const parentFilename = urlToFilename(parent?.filename);
-      this.module.hasAsyncGraph ??= this.module.isGraphAsync();
-
       if (this.module.hasAsyncGraph && !getOptionValue('--experimental-print-required-tla')) {
         throw new ERR_REQUIRE_ASYNC_MODULE(filename, parentFilename);
       }
@@ -345,14 +343,19 @@ class ModuleJob extends ModuleJobBase {
       }
       throw this.module.getError();
     } else if (status === kEvaluating || status === kEvaluated) {
+      if (this.module.hasAsyncGraph) {
+        const filename = urlToFilename(this.url);
+        const parentFilename = urlToFilename(parent?.filename);
+        throw new ERR_REQUIRE_ASYNC_MODULE(filename, parentFilename);
+      }
       // kEvaluating can show up when this is being used to deal with CJS <-> CJS cycles.
       // Allow it for now, since we only need to ban ESM <-> CJS cycles which would be
       // detected earlier during the linking phase, though the CJS handling in the ESM
       // loader won't be able to emit warnings on pending circular exports like what
       // the CJS loader does.
       // TODO(joyeecheung): remove the re-invented require() in the ESM loader and
       // always handle CJS using the CJS loader to eliminate the quirks.
-      return { __proto__: null, module: this.module, namespace: this.module.getNamespaceSync() };
+      return { __proto__: null, module: this.module, namespace: this.module.getNamespace() };
     }
     assert.fail(`Unexpected module status ${status}.`);
   }
@@ -472,7 +475,7 @@ class ModuleJobSync extends ModuleJobBase {
       }
       return { __proto__: null, module: this.module };
     } else if (status === kInstantiated) {
-      // The evaluation may have been canceled because instantiateSync() detected TLA first.
+      // The evaluation may have been canceled because instantiate() detected TLA first.
       // But when it is imported again, it's fine to re-evaluate it asynchronously.
       const timeout = -1;
       const breakOnSigint = false;
@@ -490,7 +493,7 @@ class ModuleJobSync extends ModuleJobBase {
     debug('ModuleJobSync.runSync()', this.module);
     assert(this.phase === kEvaluationPhase);
     // TODO(joyeecheung): add the error decoration logic from the async instantiate.
-    this.module.hasAsyncGraph = this.module.instantiateSync();
+    this.module.instantiate();
     // If --experimental-print-required-tla is true, proceeds to evaluation even
     // if it's async because we want to search for the TLA and help users locate
     // them.

lib/internal/vm/module.js

@@ -469,7 +469,6 @@ class SyntheticModule extends Module {
       identifier,
     });
     // A synthetic module does not have dependencies.
-    this[kWrap].link([]);
     this[kWrap].instantiate();
   }
 

src/module_wrap.cc

@@ -56,6 +56,7 @@ using v8::ObjectTemplate;
 using v8::PrimitiveArray;
 using v8::Promise;
 using v8::PromiseRejectEvent;
+using v8::PropertyCallbackInfo;
 using v8::ScriptCompiler;
 using v8::ScriptOrigin;
 using v8::String;
@@ -158,6 +159,8 @@ ModuleWrap::ModuleWrap(Realm* realm,
 
   if (!synthetic_evaluation_step->IsUndefined()) {
     synthetic_ = true;
+    // Synthetic modules have no dependencies.
+    linked_ = true;
   }
   MakeWeak();
   module_.SetWeak();
@@ -240,7 +243,7 @@ Maybe<bool> ModuleWrap::CheckUnsettledTopLevelAwait() {
     return Just(true);
   }
 
-  if (!module->IsGraphAsync()) {  // There is no TLA, no need to check.
+  if (!HasAsyncGraph()) {  // There is no TLA, no need to check.
     return Just(true);
   }
 
@@ -263,6 +266,16 @@ Maybe<bool> ModuleWrap::CheckUnsettledTopLevelAwait() {
   return Just(false);
 }
 
+bool ModuleWrap::HasAsyncGraph() {
+  if (!has_async_graph_.has_value()) {
+    Isolate* isolate = env()->isolate();
+    HandleScope scope(isolate);
+
+    has_async_graph_ = module_.Get(isolate)->IsGraphAsync();
+  }
+  return has_async_graph_.value();
+}
+
 Local<PrimitiveArray> ModuleWrap::GetHostDefinedOptions(
     Isolate* isolate, Local<Symbol> id_symbol) {
   Local<PrimitiveArray> host_defined_options =
@@ -673,35 +686,6 @@ void ModuleWrap::Link(const FunctionCallbackInfo<Value>& args) {
   dependent->linked_ = true;
 }
 
-void ModuleWrap::Instantiate(const FunctionCallbackInfo<Value>& args) {
-  Realm* realm = Realm::GetCurrent(args);
-  Isolate* isolate = args.GetIsolate();
-  ModuleWrap* obj;
-  ASSIGN_OR_RETURN_UNWRAP(&obj, args.This());
-  Local<Context> context = obj->context();
-  Local<Module> module = obj->module_.Get(isolate);
-
-  if (!obj->IsLinked()) {
-    THROW_ERR_VM_MODULE_LINK_FAILURE(realm->env(), "module is not linked");
-    return;
-  }
-
-  TryCatchScope try_catch(realm->env());
-  USE(module->InstantiateModule(
-      context, ResolveModuleCallback, ResolveSourceCallback));
-
-  if (try_catch.HasCaught() && !try_catch.HasTerminated()) {
-    CHECK(!try_catch.Message().IsEmpty());
-    CHECK(!try_catch.Exception().IsEmpty());
-    AppendExceptionLine(realm->env(),
-                        try_catch.Exception(),
-                        try_catch.Message(),
-                        ErrorHandlingMode::MODULE_ERROR);
-    try_catch.ReThrow();
-    return;
-  }
-}
-
 void ModuleWrap::Evaluate(const FunctionCallbackInfo<Value>& args) {
   Realm* realm = Realm::GetCurrent(args);
   Isolate* isolate = realm->isolate();
@@ -783,7 +767,7 @@ void ModuleWrap::Evaluate(const FunctionCallbackInfo<Value>& args) {
   }
 }
 
-void ModuleWrap::InstantiateSync(const FunctionCallbackInfo<Value>& args) {
+void ModuleWrap::Instantiate(const FunctionCallbackInfo<Value>& args) {
   Realm* realm = Realm::GetCurrent(args);
   Isolate* isolate = args.GetIsolate();
   ModuleWrap* obj;
@@ -792,6 +776,11 @@ void ModuleWrap::InstantiateSync(const FunctionCallbackInfo<Value>& args) {
   Local<Module> module = obj->module_.Get(isolate);
   Environment* env = realm->env();
 
+  if (!obj->IsLinked()) {
+    THROW_ERR_VM_MODULE_LINK_FAILURE(env, "module is not linked");
+    return;
+  }
+
   {
     TryCatchScope try_catch(env);
     USE(module->InstantiateModule(
@@ -808,10 +797,6 @@ void ModuleWrap::InstantiateSync(const FunctionCallbackInfo<Value>& args) {
       return;
     }
   }
-
-  // TODO(joyeecheung): record Module::HasTopLevelAwait() in every ModuleWrap
-  // and infer the asynchronicity from a module's children during linking.
-  args.GetReturnValue().Set(module->IsGraphAsync());
 }
 
 Maybe<void> ThrowIfPromiseRejected(Realm* realm, Local<Promise> promise) {
@@ -879,7 +864,7 @@ void ModuleWrap::EvaluateSync(const FunctionCallbackInfo<Value>& args) {
     return;
   }
 
-  if (module->IsGraphAsync()) {
+  if (obj->HasAsyncGraph()) {
     CHECK(env->options()->print_required_tla);
     auto stalled_messages =
         std::get<1>(module->GetStalledTopLevelAwaitMessages(isolate));
@@ -901,52 +886,15 @@ void ModuleWrap::EvaluateSync(const FunctionCallbackInfo<Value>& args) {
   args.GetReturnValue().Set(module->GetModuleNamespace());
 }
 
-void ModuleWrap::GetNamespaceSync(const FunctionCallbackInfo<Value>& args) {
-  Realm* realm = Realm::GetCurrent(args);
-  Isolate* isolate = args.GetIsolate();
-  ModuleWrap* obj;
-  ASSIGN_OR_RETURN_UNWRAP(&obj, args.This());
-  Local<Module> module = obj->module_.Get(isolate);
-
-  switch (module->GetStatus()) {
-    case Module::Status::kUninstantiated:
-    case Module::Status::kInstantiating:
-      return realm->env()->ThrowError(
-          "Cannot get namespace, module has not been instantiated");
-    case Module::Status::kInstantiated:
-    case Module::Status::kEvaluating:
-    case Module::Status::kEvaluated:
-    case Module::Status::kErrored:
-      break;
-  }
-
-  if (module->IsGraphAsync()) {
-    return THROW_ERR_REQUIRE_ASYNC_MODULE(realm->env(), args[0], args[1]);
-  }
-  Local<Value> result = module->GetModuleNamespace();
-  args.GetReturnValue().Set(result);
-}
-
 void ModuleWrap::GetNamespace(const FunctionCallbackInfo<Value>& args) {
   Realm* realm = Realm::GetCurrent(args);
   Isolate* isolate = args.GetIsolate();
   ModuleWrap* obj;
   ASSIGN_OR_RETURN_UNWRAP(&obj, args.This());
 
   Local<Module> module = obj->module_.Get(isolate);
-
-  switch (module->GetStatus()) {
-    case Module::Status::kUninstantiated:
-    case Module::Status::kInstantiating:
-      return realm->env()->ThrowError(
-          "cannot get namespace, module has not been instantiated");
-    case Module::Status::kInstantiated:
-    case Module::Status::kEvaluating:
-    case Module::Status::kEvaluated:
-    case Module::Status::kErrored:
-      break;
-    default:
-      UNREACHABLE();
+  if (module->GetStatus() < Module::kInstantiated) {
+    return THROW_ERR_MODULE_NOT_INSTANTIATED(realm->env());
   }
 
   Local<Value> result = module->GetModuleNamespace();
@@ -997,23 +945,28 @@ void ModuleWrap::GetStatus(const FunctionCallbackInfo<Value>& args) {
   args.GetReturnValue().Set(module->GetStatus());
 }
 
-void ModuleWrap::IsGraphAsync(const FunctionCallbackInfo<Value>& args) {
+void ModuleWrap::GetError(const FunctionCallbackInfo<Value>& args) {
   Isolate* isolate = args.GetIsolate();
   ModuleWrap* obj;
   ASSIGN_OR_RETURN_UNWRAP(&obj, args.This());
 
   Local<Module> module = obj->module_.Get(isolate);
-
-  args.GetReturnValue().Set(module->IsGraphAsync());
+  args.GetReturnValue().Set(module->GetException());
 }
 
-void ModuleWrap::GetError(const FunctionCallbackInfo<Value>& args) {
+void ModuleWrap::HasAsyncGraph(Local<Name> property,
+                               const PropertyCallbackInfo<Value>& args) {
   Isolate* isolate = args.GetIsolate();
+  Environment* env = Environment::GetCurrent(isolate);
   ModuleWrap* obj;
   ASSIGN_OR_RETURN_UNWRAP(&obj, args.This());
 
   Local<Module> module = obj->module_.Get(isolate);
-  args.GetReturnValue().Set(module->GetException());
+  if (module->GetStatus() < Module::kInstantiated) {
+    return THROW_ERR_MODULE_NOT_INSTANTIATED(env);
+  }
+
+  args.GetReturnValue().Set(obj->HasAsyncGraph());
 }
 
 // static
@@ -1417,9 +1370,7 @@ void ModuleWrap::CreatePerIsolateProperties(IsolateData* isolate_data,
 
   SetProtoMethod(isolate, tpl, "link", Link);
   SetProtoMethod(isolate, tpl, "getModuleRequests", GetModuleRequests);
-  SetProtoMethod(isolate, tpl, "instantiateSync", InstantiateSync);
   SetProtoMethod(isolate, tpl, "evaluateSync", EvaluateSync);
-  SetProtoMethod(isolate, tpl, "getNamespaceSync", GetNamespaceSync);
   SetProtoMethod(isolate, tpl, "instantiate", Instantiate);
   SetProtoMethod(isolate, tpl, "evaluate", Evaluate);
   SetProtoMethod(isolate, tpl, "setExport", SetSyntheticExport);
@@ -1429,9 +1380,12 @@ void ModuleWrap::CreatePerIsolateProperties(IsolateData* isolate_data,
       isolate, tpl, "createCachedData", CreateCachedData);
   SetProtoMethodNoSideEffect(isolate, tpl, "getNamespace", GetNamespace);
   SetProtoMethodNoSideEffect(isolate, tpl, "getStatus", GetStatus);
-  SetProtoMethodNoSideEffect(isolate, tpl, "isGraphAsync", IsGraphAsync);
   SetProtoMethodNoSideEffect(isolate, tpl, "getError", GetError);
   SetConstructorFunction(isolate, target, "ModuleWrap", tpl);
+
+  tpl->InstanceTemplate()->SetLazyDataProperty(
+      FIXED_ONE_BYTE_STRING(isolate, "hasAsyncGraph"), HasAsyncGraph);
+
   isolate_data->set_module_wrap_constructor_template(tpl);
 
   SetMethod(isolate,
@@ -1479,9 +1433,7 @@ void ModuleWrap::RegisterExternalReferences(
 
   registry->Register(Link);
   registry->Register(GetModuleRequests);
-  registry->Register(InstantiateSync);
   registry->Register(EvaluateSync);
-  registry->Register(GetNamespaceSync);
   registry->Register(Instantiate);
   registry->Register(Evaluate);
   registry->Register(SetSyntheticExport);
@@ -1491,7 +1443,7 @@ void ModuleWrap::RegisterExternalReferences(
   registry->Register(GetNamespace);
   registry->Register(GetStatus);
   registry->Register(GetError);
-  registry->Register(IsGraphAsync);
+  registry->Register(HasAsyncGraph);
 
   registry->Register(CreateRequiredModuleFacade);
 

src/module_wrap.h

@@ -121,6 +121,7 @@ class ModuleWrap : public BaseObject {
 
   v8::Local<v8::Context> context() const;
   v8::Maybe<bool> CheckUnsettledTopLevelAwait();
+  bool HasAsyncGraph();
 
   SET_MEMORY_INFO_NAME(ModuleWrap)
   SET_SELF_SIZE(ModuleWrap)
@@ -168,9 +169,7 @@ class ModuleWrap : public BaseObject {
   static void New(const v8::FunctionCallbackInfo<v8::Value>& args);
   static void GetModuleRequests(
       const v8::FunctionCallbackInfo<v8::Value>& args);
-  static void InstantiateSync(const v8::FunctionCallbackInfo<v8::Value>& args);
   static void EvaluateSync(const v8::FunctionCallbackInfo<v8::Value>& args);
-  static void GetNamespaceSync(const v8::FunctionCallbackInfo<v8::Value>& args);
   static void SetModuleSourceObject(
       const v8::FunctionCallbackInfo<v8::Value>& args);
   static void GetModuleSourceObject(
@@ -180,10 +179,12 @@ class ModuleWrap : public BaseObject {
   static void Instantiate(const v8::FunctionCallbackInfo<v8::Value>& args);
   static void Evaluate(const v8::FunctionCallbackInfo<v8::Value>& args);
   static void GetNamespace(const v8::FunctionCallbackInfo<v8::Value>& args);
-  static void IsGraphAsync(const v8::FunctionCallbackInfo<v8::Value>& args);
   static void GetStatus(const v8::FunctionCallbackInfo<v8::Value>& args);
   static void GetError(const v8::FunctionCallbackInfo<v8::Value>& args);
 
+  static void HasAsyncGraph(v8::Local<v8::Name> property,
+                            const v8::PropertyCallbackInfo<v8::Value>& args);
+
   static void SetImportModuleDynamicallyCallback(
       const v8::FunctionCallbackInfo<v8::Value>& args);
   static void SetInitializeImportMetaObjectCallback(
@@ -219,6 +220,9 @@ class ModuleWrap : public BaseObject {
   contextify::ContextifyContext* contextify_context_ = nullptr;
   bool synthetic_ = false;
   bool linked_ = false;
+  // This depends on the module to be instantiated so it begins with a
+  // nullopt value.
+  std::optional<bool> has_async_graph_ = std::nullopt;
   int module_hash_;
 };
 

src/node_errors.h

@@ -110,6 +110,7 @@ void OOMErrorHandler(const char* location, const v8::OOMDetails& details);
   V(ERR_MISSING_PASSPHRASE, TypeError)                                         \
   V(ERR_MISSING_PLATFORM_FOR_WORKER, Error)                                    \
   V(ERR_MODULE_NOT_FOUND, Error)                                               \
+  V(ERR_MODULE_NOT_INSTANTIATED, Error)                                        \
   V(ERR_MODULE_LINK_MISMATCH, TypeError)                                       \
   V(ERR_NON_CONTEXT_AWARE_DISABLED, Error)                                     \
   V(ERR_OPERATION_FAILED, TypeError)                                           \
@@ -229,6 +230,7 @@ ERRORS_WITH_CODE(V)
   V(ERR_MISSING_PLATFORM_FOR_WORKER,                                           \
     "The V8 platform used by this instance of Node does not support "          \
     "creating Workers")                                                        \
+  V(ERR_MODULE_NOT_INSTANTIATED, "Module is not instantiated")                 \
   V(ERR_NON_CONTEXT_AWARE_DISABLED,                                            \
     "Loading non context-aware native addons has been disabled")               \
   V(ERR_SCRIPT_EXECUTION_INTERRUPTED,                                          \