Repository: nodejs/node
Pull request: module: prevent main thread exiting before esm worker ends

This fix ensures the main thread does not terminate before an ESM worker's `handleMessage` function completes by moving the atomic lock release to the end of the function. As far as I can see, there doesn’t seem to be any necessary processing after the lock release.

#### Problem
The `test-esm-loader-spawn-promisified.mjs` test occasionally fails in debug builds. If processing after the lock release exceeds the stack limit, it results in a fatal error:

```
✖ can predetermine the format in the custom loader resolve hook (162.586375ms)
  AssertionError [ERR_ASSERTION]: Expected values to be strictly equal:
  + actual - expected
  
  + '\n' +
  +   '\n' +
  +   '#\n' +
  +   '# Fatal error in ../../deps/v8/src/api/api.cc, line 5021\n' +
  +   '# Debug check failed: !i_isolate->is_execution_terminating().\n' +
... 
```
Even if it fails in a debug build, it is desirable for the error to be a JavaScript error, such as Maximum call stack size exceeded, rather than a debug check error.

Although it involves modifying the internal code, the issue can be reproduced as follows:
``` js
// Insert code that intentionally exceeds the stack limit at line 220 of lib/internal/modules/esm/worker.js before the fix.
    if(method === "load" && args[0].includes("imported.mjs")) {
      function func() {
          func();
      }
      func();
    } 

// index.mjs
await import("./imported.mjs")
console.log("done")

// run index.mjs with any loader and any imported.mjs
```

#### Test
Testing this precisely is difficult due to timing sensitivity. A new test verifies that a deliberate stack overflow results in the correct `Maximum call stack size exceeded` error, ensuring the lock behavior is correct.


Changed files:
filename: lib/internal/modules/esm/worker.js
patch: @@ -215,8 +215,6 @@ async function customizedModuleWorker(lock, syncCommPort, errorHandler) {
       (port ?? syncCommPort).postMessage(wrapMessage('error', exception));
     }
 
-    AtomicsAdd(lock, WORKER_TO_MAIN_THREAD_NOTIFICATION, 1);
-    AtomicsNotify(lock, WORKER_TO_MAIN_THREAD_NOTIFICATION);
     if (shouldRemoveGlobalErrorHandler) {
       process.off('uncaughtException', errorHandler);
     }
@@ -225,6 +223,10 @@ async function customizedModuleWorker(lock, syncCommPort, errorHandler) {
     // We keep checking for new messages to not miss any.
     clearImmediate(immediate);
     immediate = setImmediate(checkForMessages).unref();
+    // To prevent the main thread from terminating before this function completes after unlocking,
+    // the following process is executed at the end of the function.
+    AtomicsAdd(lock, WORKER_TO_MAIN_THREAD_NOTIFICATION, 1);
+    AtomicsNotify(lock, WORKER_TO_MAIN_THREAD_NOTIFICATION);
   }
 }
 

filename: test/es-module/test-esm-loader-spawn-promisified.mjs
patch: @@ -285,4 +285,20 @@ describe('Loader hooks parsing modules', { concurrency: !process.env.TEST_PARALL
     assert.strictEqual(code, 0);
     assert.strictEqual(signal, null);
   });
+
+  it('throw maximum call stack error on the loader', async () => {
+    const { code, signal, stdout, stderr } = await spawnPromisified(execPath, [
+      '--no-warnings',
+      '--experimental-loader',
+      fixtures.fileURL('/es-module-loaders/hooks-custom.mjs'),
+      '--input-type=module',
+      '--eval',
+      'await import("esmHook/maximumCallStack.mjs")',
+    ]);
+
+    assert(stderr.includes('Maximum call stack size exceeded'));
+    assert.strictEqual(stdout, '');
+    assert.strictEqual(code, 1);
+    assert.strictEqual(signal, null);
+  });
 });

filename: test/fixtures/es-module-loaders/hooks-custom.mjs
patch: @@ -105,5 +105,12 @@ export function load(url, context, next) {
     };
   }
 
+  if (url.endsWith('esmHook/maximumCallStack.mjs')) {
+    function recurse() {
+      recurse();
+    }
+    recurse();
+  }
+
   return next(url);
 }