module: handle instantiated async module jobs in require(esm)

When require(esm) encounters a cached module job that is instantiated
but not yet evaluated, run the evaluation. This catches an edge case
previously missed in nodejs#57187.

PR-URL: nodejs#58067
Fixes: nodejs#58061
Reviewed-By: Jacob Smith <[email protected]>

Author: joyeecheung

lib/internal/modules/esm/loader.js

@@ -378,7 +378,7 @@ class ModuleLoader {
         throw new ERR_REQUIRE_ASYNC_MODULE(filename, parentFilename);
       }
       const status = job.module.getStatus();
-      debug('Module status', filename, status);
+      debug('Module status', job, status);
       if (status === kEvaluated) {
         return { wrap: job.module, namespace: job.module.getNamespaceSync(filename, parentFilename) };
       } else if (status === kInstantiated) {

lib/internal/modules/esm/module_job.js

@@ -22,7 +22,14 @@ let debug = require('internal/util/debuglog').debuglog('esm', (fn) => {
   debug = fn;
 });
 
-const { ModuleWrap, kEvaluationPhase, kInstantiated } = internalBinding('module_wrap');
+const {
+  ModuleWrap,
+  kErrored,
+  kEvaluated,
+  kEvaluationPhase,
+  kInstantiated,
+  kUninstantiated,
+} = internalBinding('module_wrap');
 const {
   privateSymbols: {
     entry_point_module_private_symbol,
@@ -280,17 +287,34 @@ class ModuleJob extends ModuleJobBase {
   runSync(parent) {
     assert(this.phase === kEvaluationPhase);
     assert(this.module instanceof ModuleWrap);
-    if (this.instantiated !== undefined) {
-      return { __proto__: null, module: this.module };
+    let status = this.module.getStatus();
+
+    debug('ModuleJob.runSync', this.module);
+    // FIXME(joyeecheung): this cannot fully handle < kInstantiated. Make the linking
+    // fully synchronous instead.
+    if (status === kUninstantiated) {
+      this.module.async = this.module.instantiateSync();
+      status = this.module.getStatus();
     }
+    if (status === kInstantiated || status === kErrored) {
+      const filename = urlToFilename(this.url);
+      const parentFilename = urlToFilename(parent?.filename);
+      this.module.async ??= this.module.isGraphAsync();
 
-    this.module.instantiate();
-    this.instantiated = PromiseResolve();
-    setHasStartedUserESMExecution();
-    const filename = urlToFilename(this.url);
-    const parentFilename = urlToFilename(parent?.filename);
-    const namespace = this.module.evaluateSync(filename, parentFilename);
-    return { __proto__: null, module: this.module, namespace };
+      if (this.module.async && !getOptionValue('--experimental-print-required-tla')) {
+        throw new ERR_REQUIRE_ASYNC_MODULE(filename, parentFilename);
+      }
+      if (status === kInstantiated) {
+        setHasStartedUserESMExecution();
+        const namespace = this.module.evaluateSync(filename, parentFilename);
+        return { __proto__: null, module: this.module, namespace };
+      }
+      throw this.module.getError();
+
+    } else if (status === kEvaluated) {
+      return { __proto__: null, module: this.module, namespace: this.module.getNamespaceSync() };
+    }
+    assert.fail(`Unexpected module status ${status}.`);
   }
 
   async run(isEntryPoint = false) {

src/debug_utils.h

@@ -52,6 +52,7 @@ void NODE_EXTERN_PRIVATE FWrite(FILE* file, const std::string& str);
   V(NGTCP2_DEBUG)                                                              \
   V(SEA)                                                                       \
   V(WASI)                                                                      \
+  V(MODULE)                                                                    \
   V(MKSNAPSHOT)                                                                \
   V(SNAPSHOT_SERDES)                                                           \
   V(PERMISSION_MODEL)                                                          \

src/module_wrap.cc

@@ -815,6 +815,16 @@ void ModuleWrap::GetStatus(const FunctionCallbackInfo<Value>& args) {
   args.GetReturnValue().Set(module->GetStatus());
 }
 
+void ModuleWrap::IsGraphAsync(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());
+}
+
 void ModuleWrap::GetError(const FunctionCallbackInfo<Value>& args) {
   Isolate* isolate = args.GetIsolate();
   ModuleWrap* obj;
@@ -1171,6 +1181,7 @@ 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);
   isolate_data->set_module_wrap_constructor_template(tpl);
@@ -1227,6 +1238,7 @@ void ModuleWrap::RegisterExternalReferences(
   registry->Register(GetNamespace);
   registry->Register(GetStatus);
   registry->Register(GetError);
+  registry->Register(IsGraphAsync);
 
   registry->Register(CreateRequiredModuleFacade);
 

src/module_wrap.h

@@ -111,6 +111,7 @@ 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);
 

test/es-module/test-require-module-instantiated.mjs

@@ -0,0 +1,4 @@
+import '../common/index.mjs';
+import assert from 'node:assert';
+import { b, c } from '../fixtures/es-modules/require-module-instantiated/a.mjs';
+assert.strictEqual(b, c);

test/fixtures/es-modules/require-module-instantiated/a.mjs

@@ -0,0 +1,2 @@
+export { default as b } from './b.cjs';
+export { default as c } from './c.mjs';

test/fixtures/es-modules/require-module-instantiated/b.cjs

@@ -0,0 +1 @@
+module.exports = require('./c.mjs');

test/fixtures/es-modules/require-module-instantiated/c.mjs

@@ -0,0 +1,3 @@
+const foo = 1;
+export default foo;
+export { foo as 'module.exports' };