module: prevent main thread exiting before esm worker ends

islandryu · ruyadorno · commit 8a5429c9b341 · 2025-01-05T13:05:26.000-05:00
PR-URL: #56183 Reviewed-By: Matteo Collina <matteo.collina@gmail.com> Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com> Reviewed-By: Jacob Smith <jacob@frende.me>
diff --git a/lib/internal/modules/esm/worker.js b/lib/internal/modules/esm/worker.js
@@ -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);
   }
 }
 
diff --git a/test/es-module/test-esm-loader-spawn-promisified.mjs b/test/es-module/test-esm-loader-spawn-promisified.mjs
@@ -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);
+  });
 });
diff --git a/test/fixtures/es-module-loaders/hooks-custom.mjs b/test/fixtures/es-module-loaders/hooks-custom.mjs
@@ -105,5 +105,12 @@ export function load(url, context, next) {
     };
   }
 
+  if (url.endsWith('esmHook/maximumCallStack.mjs')) {
+    function recurse() {
+      recurse();
+    }
+    recurse();
+  }
+
   return next(url);
 }

Original file line number	Diff line number	Diff line change
`@@ -215,8 +215,6 @@ async function customizedModuleWorker(lock, syncCommPort, errorHandler) {`
`215`	`215`	`(port ?? syncCommPort).postMessage(wrapMessage('error', exception));`
`216`	`216`	`}`
`217`	`217`
`218`		`- AtomicsAdd(lock, WORKER_TO_MAIN_THREAD_NOTIFICATION, 1);`
`219`		`- AtomicsNotify(lock, WORKER_TO_MAIN_THREAD_NOTIFICATION);`
`220`	`218`	`if (shouldRemoveGlobalErrorHandler) {`
`221`	`219`	`process.off('uncaughtException', errorHandler);`
`222`	`220`	`}`
`@@ -225,6 +223,10 @@ async function customizedModuleWorker(lock, syncCommPort, errorHandler) {`
`225`	`223`	`// We keep checking for new messages to not miss any.`
`226`	`224`	`clearImmediate(immediate);`
`227`	`225`	`immediate = setImmediate(checkForMessages).unref();`
	`226`	`+ // To prevent the main thread from terminating before this function completes after unlocking,`
	`227`	`+ // the following process is executed at the end of the function.`
	`228`	`+ AtomicsAdd(lock, WORKER_TO_MAIN_THREAD_NOTIFICATION, 1);`
	`229`	`+ AtomicsNotify(lock, WORKER_TO_MAIN_THREAD_NOTIFICATION);`
`228`	`230`	`}`
`229`	`231`	`}`
`230`	`232`
Original file line number	Diff line number	Diff line change
`@@ -105,5 +105,12 @@ export function load(url, context, next) {`
`105`	`105`	`};`
`106`	`106`	`}`
`107`	`107`
	`108`	`+ if (url.endsWith('esmHook/maximumCallStack.mjs')) {`
	`109`	`+ function recurse() {`
	`110`	`+ recurse();`
	`111`	`+ }`
	`112`	`+ recurse();`
	`113`	`+ }`
	`114`	`+`
`108`	`115`	`return next(url);`
`109`	`116`	`}`