From a011799fab80655469cc1ee011172e549186087f Mon Sep 17 00:00:00 2001 From: Antoine du Hamel Date: Thu, 13 Apr 2023 19:07:51 +0200 Subject: [PATCH 1/2] esm: propagate `process.exit` from the loader thread to the main thread --- lib/internal/modules/esm/hooks.js | 2 ++ lib/internal/modules/esm/worker.js | 13 ++++++++++ test/es-module/test-esm-loader-hooks.mjs | 31 ++++++++++++++++++++++++ 3 files changed, 46 insertions(+) diff --git a/lib/internal/modules/esm/hooks.js b/lib/internal/modules/esm/hooks.js index 7df6672582c50d..58cc434430a8f6 100644 --- a/lib/internal/modules/esm/hooks.js +++ b/lib/internal/modules/esm/hooks.js @@ -503,6 +503,7 @@ class HooksProxy { }, }); this.#worker.unref(); // ! Allows the process to eventually exit. + this.#worker.on('exit', process.exit); } #waitForWorker() { @@ -512,6 +513,7 @@ class HooksProxy { debug('wait for signal from worker'); AtomicsWait(this.#lock, WORKER_TO_MAIN_THREAD_NOTIFICATION, 0); const response = this.#worker.receiveMessageSync(); + if (response.message.status === 'exit') return; const { preloadScripts } = this.#unwrapMessage(response); this.#executePreloadScripts(preloadScripts); } diff --git a/lib/internal/modules/esm/worker.js b/lib/internal/modules/esm/worker.js index 8a1ef9add7060f..630db9a75c6a81 100644 --- a/lib/internal/modules/esm/worker.js +++ b/lib/internal/modules/esm/worker.js @@ -66,6 +66,19 @@ async function customizedModuleWorker(lock, syncCommPort, errorHandler) { let hooks, preloadScripts, initializationError; let hasInitializationError = false; + { + // If a custom hook is calling `process.exit`, we should wake up the main thread + // so it can detect the exit event. + const { exit } = process; + process.exit = function(code) { + syncCommPort.postMessage(wrapMessage('exit')); + AtomicsAdd(lock, WORKER_TO_MAIN_THREAD_NOTIFICATION, 1); + AtomicsNotify(lock, WORKER_TO_MAIN_THREAD_NOTIFICATION); + return ReflectApply(exit, this, arguments); + }; + } + + try { initializeESM(); const initResult = await initializeHooks(); diff --git a/test/es-module/test-esm-loader-hooks.mjs b/test/es-module/test-esm-loader-hooks.mjs index 76d55fd7815cdc..94379a0825c652 100644 --- a/test/es-module/test-esm-loader-hooks.mjs +++ b/test/es-module/test-esm-loader-hooks.mjs @@ -195,4 +195,35 @@ describe('Loader hooks', { concurrency: true }, () => { assert.strictEqual(code, 0); assert.strictEqual(signal, null); }); + + it('should be fine to call `process.exit` from a custom hook', async () => { + const { code, signal, stdout, stderr } = await spawnPromisified(execPath, [ + '--no-warnings', + '--experimental-import-meta-resolve', + '--experimental-loader', + 'data:text/javascript,export function resolve(a,b,next){if(a==="exit")process.exit(42);return next(a,b)}', + '--input-type=module', + '--eval', + 'import.meta.resolve("exit")', + ]); + + assert.strictEqual(stderr, ''); + assert.strictEqual(stdout, ''); + assert.strictEqual(code, 42); + assert.strictEqual(signal, null); + }); + + it('should be fine to call `process.exit` from the loader thread top-level', async () => { + const { code, signal, stdout, stderr } = await spawnPromisified(execPath, [ + '--no-warnings', + '--experimental-loader', + 'data:text/javascript,process.exit(42)', + fixtures.path('empty.js'), + ]); + + assert.strictEqual(stderr, ''); + assert.strictEqual(stdout, ''); + assert.strictEqual(code, 42); + assert.strictEqual(signal, null); + }); }); From 9231ae16aaeaa6ef3113acae6be918556ba0ff1c Mon Sep 17 00:00:00 2001 From: Antoine du Hamel Date: Fri, 14 Apr 2023 11:49:43 +0200 Subject: [PATCH 2/2] fixup! esm: propagate `process.exit` from the loader thread to the main thread --- lib/internal/modules/esm/hooks.js | 4 +++- lib/internal/modules/esm/worker.js | 2 +- test/es-module/test-esm-loader-hooks.mjs | 23 ++++++++++++++++++++--- 3 files changed, 24 insertions(+), 5 deletions(-) diff --git a/lib/internal/modules/esm/hooks.js b/lib/internal/modules/esm/hooks.js index 58cc434430a8f6..584430265bb61a 100644 --- a/lib/internal/modules/esm/hooks.js +++ b/lib/internal/modules/esm/hooks.js @@ -513,7 +513,7 @@ class HooksProxy { debug('wait for signal from worker'); AtomicsWait(this.#lock, WORKER_TO_MAIN_THREAD_NOTIFICATION, 0); const response = this.#worker.receiveMessageSync(); - if (response.message.status === 'exit') return; + if (response.message.status === 'exit') { return; } const { preloadScripts } = this.#unwrapMessage(response); this.#executePreloadScripts(preloadScripts); } @@ -594,6 +594,8 @@ class HooksProxy { debug('got sync response from worker', { method, args }); if (response.message.status === 'never-settle') { process.exit(kUnfinishedTopLevelAwait); + } else if (response.message.status === 'exit') { + process.exit(response.message.body); } return this.#unwrapMessage(response); } diff --git a/lib/internal/modules/esm/worker.js b/lib/internal/modules/esm/worker.js index 630db9a75c6a81..e343fbe03f1748 100644 --- a/lib/internal/modules/esm/worker.js +++ b/lib/internal/modules/esm/worker.js @@ -71,7 +71,7 @@ async function customizedModuleWorker(lock, syncCommPort, errorHandler) { // so it can detect the exit event. const { exit } = process; process.exit = function(code) { - syncCommPort.postMessage(wrapMessage('exit')); + syncCommPort.postMessage(wrapMessage('exit', code ?? process.exitCode)); AtomicsAdd(lock, WORKER_TO_MAIN_THREAD_NOTIFICATION, 1); AtomicsNotify(lock, WORKER_TO_MAIN_THREAD_NOTIFICATION); return ReflectApply(exit, this, arguments); diff --git a/test/es-module/test-esm-loader-hooks.mjs b/test/es-module/test-esm-loader-hooks.mjs index 94379a0825c652..59632fec26a47b 100644 --- a/test/es-module/test-esm-loader-hooks.mjs +++ b/test/es-module/test-esm-loader-hooks.mjs @@ -196,15 +196,32 @@ describe('Loader hooks', { concurrency: true }, () => { assert.strictEqual(signal, null); }); - it('should be fine to call `process.exit` from a custom hook', async () => { + it('should be fine to call `process.exit` from a custom async hook', async () => { const { code, signal, stdout, stderr } = await spawnPromisified(execPath, [ '--no-warnings', '--experimental-import-meta-resolve', '--experimental-loader', - 'data:text/javascript,export function resolve(a,b,next){if(a==="exit")process.exit(42);return next(a,b)}', + 'data:text/javascript,export function load(a,b,next){if(a==="data:exit")process.exit(42);return next(a,b)}', '--input-type=module', '--eval', - 'import.meta.resolve("exit")', + 'import "data:exit"', + ]); + + assert.strictEqual(stderr, ''); + assert.strictEqual(stdout, ''); + assert.strictEqual(code, 42); + assert.strictEqual(signal, null); + }); + + it('should be fine to call `process.exit` from a custom sync hook', async () => { + const { code, signal, stdout, stderr } = await spawnPromisified(execPath, [ + '--no-warnings', + '--experimental-import-meta-resolve', + '--experimental-loader', + 'data:text/javascript,export function resolve(a,b,next){if(a==="exit:")process.exit(42);return next(a,b)}', + '--input-type=module', + '--eval', + 'import "data:text/javascript,import.meta.resolve(%22exit:%22)"', ]); assert.strictEqual(stderr, '');