From b828eaccd8dcad8789eb38e1ad1331d672b80bc9 Mon Sep 17 00:00:00 2001 From: Gang Chen Date: Mon, 3 Jan 2022 23:54:03 +0800 Subject: [PATCH 01/11] module: `process.exit()` should result in exit code 0 --- .../modules/esm/handle_process_exit.js | 10 ++++++++++ lib/internal/modules/run_main.js | 11 +++++------ lib/internal/process/per_thread.js | 6 ++++++ test/es-module/test-esm-tla-unfinished.mjs | 18 ++++++++++++++++++ test/fixtures/es-modules/tla/process-exit.mjs | 1 + .../unresolved-with-worker-process-exit.mjs | 10 ++++++++++ 6 files changed, 50 insertions(+), 6 deletions(-) create mode 100644 lib/internal/modules/esm/handle_process_exit.js create mode 100644 test/fixtures/es-modules/tla/process-exit.mjs create mode 100644 test/fixtures/es-modules/tla/unresolved-with-worker-process-exit.mjs diff --git a/lib/internal/modules/esm/handle_process_exit.js b/lib/internal/modules/esm/handle_process_exit.js new file mode 100644 index 00000000000000..2f7e6438260c81 --- /dev/null +++ b/lib/internal/modules/esm/handle_process_exit.js @@ -0,0 +1,10 @@ +'use strict'; + +function handleProcessExit() { + if (process.exitCode === undefined) + process.exitCode = 13; +} + +module.exports = { + handleProcessExit, +}; diff --git a/lib/internal/modules/run_main.js b/lib/internal/modules/run_main.js index 9a0263024144fb..5c1cc943ebe9ae 100644 --- a/lib/internal/modules/run_main.js +++ b/lib/internal/modules/run_main.js @@ -8,6 +8,9 @@ const CJSLoader = require('internal/modules/cjs/loader'); const { Module, toRealPath, readPackageScope } = CJSLoader; const { getOptionValue } = require('internal/options'); const path = require('path'); +const { + handleProcessExit +} = require('internal/modules/esm/handle_process_exit'); function resolveMainPath(main) { // Note extension resolution for the main entry point can be deprecated in a @@ -56,15 +59,11 @@ async function handleMainPromise(promise) { // Handle a Promise from running code that potentially does Top-Level Await. // In that case, it makes sense to set the exit code to a specific non-zero // value if the main code never finishes running. - function handler() { - if (process.exitCode === undefined) - process.exitCode = 13; - } - process.on('exit', handler); + process.on('exit', handleProcessExit); try { return await promise; } finally { - process.off('exit', handler); + process.off('exit', handleProcessExit); } } diff --git a/lib/internal/process/per_thread.js b/lib/internal/process/per_thread.js index a63217d9b3955a..7fc6212a93883f 100644 --- a/lib/internal/process/per_thread.js +++ b/lib/internal/process/per_thread.js @@ -48,6 +48,10 @@ const { } = require('internal/validators'); const constants = internalBinding('constants').os.signals; +const { + handleProcessExit +} = require('internal/modules/esm/handle_process_exit'); + const kInternal = Symbol('internal properties'); function assert(x, msg) { @@ -175,6 +179,8 @@ function wrapProcessMethods(binding) { memoryUsage.rss = rss; function exit(code) { + process.off('exit', handleProcessExit); + if (code || code === 0) process.exitCode = code; diff --git a/test/es-module/test-esm-tla-unfinished.mjs b/test/es-module/test-esm-tla-unfinished.mjs index 7d35c86285ac81..d7658c19e98e1c 100644 --- a/test/es-module/test-esm-tla-unfinished.mjs +++ b/test/es-module/test-esm-tla-unfinished.mjs @@ -80,3 +80,21 @@ import fixtures from '../common/fixtures.js'; assert.deepStrictEqual([status, stdout], [1, '']); assert.match(stderr, /Error: Xyz/); } + +{ + // Calling process.exit() in .mjs should return status 0 + const { status, stdout, stderr } = child_process.spawnSync( + process.execPath, + [fixtures.path('es-modules/tla/process-exit.mjs')], + { encoding: 'utf8' }); + assert.deepStrictEqual([status, stdout, stderr], [0, '', '']); +} + +{ + // Calling process.exit() in worker thread shouldn't influence main thread + const { status, stdout, stderr } = child_process.spawnSync( + process.execPath, + [fixtures.path('es-modules/tla/unresolved-with-worker-process-exit.mjs')], + { encoding: 'utf8' }); + assert.deepStrictEqual([status, stdout, stderr], [13, '', '']); +} diff --git a/test/fixtures/es-modules/tla/process-exit.mjs b/test/fixtures/es-modules/tla/process-exit.mjs new file mode 100644 index 00000000000000..37dd593a2b4aae --- /dev/null +++ b/test/fixtures/es-modules/tla/process-exit.mjs @@ -0,0 +1 @@ +process.exit(); \ No newline at end of file diff --git a/test/fixtures/es-modules/tla/unresolved-with-worker-process-exit.mjs b/test/fixtures/es-modules/tla/unresolved-with-worker-process-exit.mjs new file mode 100644 index 00000000000000..3c0bc75167423a --- /dev/null +++ b/test/fixtures/es-modules/tla/unresolved-with-worker-process-exit.mjs @@ -0,0 +1,10 @@ +import { Worker } from 'worker_threads'; + +// Do not use isMainThread so that this test itself can be run inside a Worker. +if (!process.env.HAS_STARTED_WORKER) { + process.env.HAS_STARTED_WORKER = 1; + new Worker(import.meta.url.slice('file://'.length)); + await new Promise(() => {}); +} else { + process.exit() +} From 5ebf9b0d73be6000c30302955b597a0bad8a4a1e Mon Sep 17 00:00:00 2001 From: Gang Chen <13298548+MoonBall@users.noreply.github.com> Date: Tue, 4 Jan 2022 07:27:57 +0800 Subject: [PATCH 02/11] Update test/fixtures/es-modules/tla/unresolved-with-worker-process-exit.mjs Co-authored-by: Antoine du Hamel --- .../es-modules/tla/unresolved-with-worker-process-exit.mjs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/fixtures/es-modules/tla/unresolved-with-worker-process-exit.mjs b/test/fixtures/es-modules/tla/unresolved-with-worker-process-exit.mjs index 3c0bc75167423a..c4ec8d2f5d1421 100644 --- a/test/fixtures/es-modules/tla/unresolved-with-worker-process-exit.mjs +++ b/test/fixtures/es-modules/tla/unresolved-with-worker-process-exit.mjs @@ -3,7 +3,7 @@ import { Worker } from 'worker_threads'; // Do not use isMainThread so that this test itself can be run inside a Worker. if (!process.env.HAS_STARTED_WORKER) { process.env.HAS_STARTED_WORKER = 1; - new Worker(import.meta.url.slice('file://'.length)); + new Worker(new URL(import.meta.url)); await new Promise(() => {}); } else { process.exit() From fab1eed97331a60e1961325b0b693701a0c55d68 Mon Sep 17 00:00:00 2001 From: Gang Chen <13298548+MoonBall@users.noreply.github.com> Date: Tue, 4 Jan 2022 07:28:04 +0800 Subject: [PATCH 03/11] Update test/fixtures/es-modules/tla/process-exit.mjs Co-authored-by: Antoine du Hamel --- test/fixtures/es-modules/tla/process-exit.mjs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/fixtures/es-modules/tla/process-exit.mjs b/test/fixtures/es-modules/tla/process-exit.mjs index 37dd593a2b4aae..78e86b018e7fbb 100644 --- a/test/fixtures/es-modules/tla/process-exit.mjs +++ b/test/fixtures/es-modules/tla/process-exit.mjs @@ -1 +1 @@ -process.exit(); \ No newline at end of file +process.exit(); From 4e6f2c06f2728c189ef8d24a05feadde894b4795 Mon Sep 17 00:00:00 2001 From: Gang Chen <13298548+MoonBall@users.noreply.github.com> Date: Tue, 4 Jan 2022 07:28:12 +0800 Subject: [PATCH 04/11] Update lib/internal/modules/run_main.js Co-authored-by: Antoine du Hamel --- lib/internal/modules/run_main.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/internal/modules/run_main.js b/lib/internal/modules/run_main.js index 5c1cc943ebe9ae..6cc1e0e74bc2dd 100644 --- a/lib/internal/modules/run_main.js +++ b/lib/internal/modules/run_main.js @@ -9,7 +9,7 @@ const { Module, toRealPath, readPackageScope } = CJSLoader; const { getOptionValue } = require('internal/options'); const path = require('path'); const { - handleProcessExit + handleProcessExit, } = require('internal/modules/esm/handle_process_exit'); function resolveMainPath(main) { From dbf136c542654674a456eb342af783ad126aad68 Mon Sep 17 00:00:00 2001 From: Gang Chen <13298548+MoonBall@users.noreply.github.com> Date: Tue, 4 Jan 2022 07:28:17 +0800 Subject: [PATCH 05/11] Update lib/internal/process/per_thread.js Co-authored-by: Antoine du Hamel --- lib/internal/process/per_thread.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/internal/process/per_thread.js b/lib/internal/process/per_thread.js index 7fc6212a93883f..709bcb7b137639 100644 --- a/lib/internal/process/per_thread.js +++ b/lib/internal/process/per_thread.js @@ -49,7 +49,7 @@ const { const constants = internalBinding('constants').os.signals; const { - handleProcessExit + handleProcessExit, } = require('internal/modules/esm/handle_process_exit'); const kInternal = Symbol('internal properties'); From 24bc42cfa81e1fddd1831ecf58de48a22b22e127 Mon Sep 17 00:00:00 2001 From: Gang Chen <13298548+MoonBall@users.noreply.github.com> Date: Tue, 4 Jan 2022 07:30:25 +0800 Subject: [PATCH 06/11] Update test/fixtures/es-modules/tla/unresolved-with-worker-process-exit.mjs Co-authored-by: Mestery --- .../es-modules/tla/unresolved-with-worker-process-exit.mjs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/fixtures/es-modules/tla/unresolved-with-worker-process-exit.mjs b/test/fixtures/es-modules/tla/unresolved-with-worker-process-exit.mjs index c4ec8d2f5d1421..1860e95a73dd75 100644 --- a/test/fixtures/es-modules/tla/unresolved-with-worker-process-exit.mjs +++ b/test/fixtures/es-modules/tla/unresolved-with-worker-process-exit.mjs @@ -6,5 +6,5 @@ if (!process.env.HAS_STARTED_WORKER) { new Worker(new URL(import.meta.url)); await new Promise(() => {}); } else { - process.exit() + process.exit(); } From adcf3742baf1234dd02183faf766ff1d8073ef23 Mon Sep 17 00:00:00 2001 From: Gang Chen <13298548+MoonBall@users.noreply.github.com> Date: Tue, 4 Jan 2022 07:30:58 +0800 Subject: [PATCH 07/11] Update lib/internal/modules/esm/handle_process_exit.js Co-authored-by: Jordan Harband --- lib/internal/modules/esm/handle_process_exit.js | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/lib/internal/modules/esm/handle_process_exit.js b/lib/internal/modules/esm/handle_process_exit.js index 2f7e6438260c81..10b68a8b28db00 100644 --- a/lib/internal/modules/esm/handle_process_exit.js +++ b/lib/internal/modules/esm/handle_process_exit.js @@ -1,8 +1,7 @@ 'use strict'; function handleProcessExit() { - if (process.exitCode === undefined) - process.exitCode = 13; + process.exitCode ??= 13; } module.exports = { From 73e0b71f37193cd689ed3faf84f3c9d49a07eab7 Mon Sep 17 00:00:00 2001 From: Gang Chen Date: Tue, 4 Jan 2022 08:04:42 +0800 Subject: [PATCH 08/11] use isMainThread --- .../es-modules/tla/unresolved-with-worker-process-exit.mjs | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/test/fixtures/es-modules/tla/unresolved-with-worker-process-exit.mjs b/test/fixtures/es-modules/tla/unresolved-with-worker-process-exit.mjs index 1860e95a73dd75..8d65baa2b59cce 100644 --- a/test/fixtures/es-modules/tla/unresolved-with-worker-process-exit.mjs +++ b/test/fixtures/es-modules/tla/unresolved-with-worker-process-exit.mjs @@ -1,8 +1,6 @@ -import { Worker } from 'worker_threads'; +import { Worker, isMainThread } from 'worker_threads'; -// Do not use isMainThread so that this test itself can be run inside a Worker. -if (!process.env.HAS_STARTED_WORKER) { - process.env.HAS_STARTED_WORKER = 1; +if (isMainThread) { new Worker(new URL(import.meta.url)); await new Promise(() => {}); } else { From d58b5b9c93d3f26f5b41ebc3c6fd88a5e5a8964c Mon Sep 17 00:00:00 2001 From: Gang Chen Date: Tue, 4 Jan 2022 15:30:25 +0800 Subject: [PATCH 09/11] add `handle_process_exit` to bootstrap modules --- test/parallel/test-bootstrap-modules.js | 1 + 1 file changed, 1 insertion(+) diff --git a/test/parallel/test-bootstrap-modules.js b/test/parallel/test-bootstrap-modules.js index 9a687eede28ca9..cd1a503837191e 100644 --- a/test/parallel/test-bootstrap-modules.js +++ b/test/parallel/test-bootstrap-modules.js @@ -79,6 +79,7 @@ const expectedModules = new Set([ 'NativeModule internal/modules/esm/resolve', 'NativeModule internal/modules/esm/initialize_import_meta', 'NativeModule internal/modules/esm/translators', + 'NativeModule internal/modules/esm/handle_process_exit', 'NativeModule internal/process/esm_loader', 'NativeModule internal/options', 'NativeModule internal/perf/event_loop_delay', From d7bb21fcacf9915c36eff5de283688123a4b5a84 Mon Sep 17 00:00:00 2001 From: Gang Chen Date: Tue, 4 Jan 2022 17:10:50 +0800 Subject: [PATCH 10/11] move comment to handle_process_exit file --- lib/internal/modules/esm/handle_process_exit.js | 4 ++++ lib/internal/modules/run_main.js | 3 --- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/lib/internal/modules/esm/handle_process_exit.js b/lib/internal/modules/esm/handle_process_exit.js index 10b68a8b28db00..8453c7d88c6473 100644 --- a/lib/internal/modules/esm/handle_process_exit.js +++ b/lib/internal/modules/esm/handle_process_exit.js @@ -1,5 +1,9 @@ 'use strict'; +// Handle a Promise from running code that potentially does Top-Level Await. +// In that case, it makes sense to set the exit code to a specific non-zero +// value if the main code never finishes running. +// The original purpose was https://github.com/nodejs/node/pull/34640 function handleProcessExit() { process.exitCode ??= 13; } diff --git a/lib/internal/modules/run_main.js b/lib/internal/modules/run_main.js index 6cc1e0e74bc2dd..924c4836bb2aa2 100644 --- a/lib/internal/modules/run_main.js +++ b/lib/internal/modules/run_main.js @@ -56,9 +56,6 @@ function runMainESM(mainPath) { } async function handleMainPromise(promise) { - // Handle a Promise from running code that potentially does Top-Level Await. - // In that case, it makes sense to set the exit code to a specific non-zero - // value if the main code never finishes running. process.on('exit', handleProcessExit); try { return await promise; From 0951516f7073ec878217465cf07b1c7931f1ecfb Mon Sep 17 00:00:00 2001 From: Gang Chen <13298548+MoonBall@users.noreply.github.com> Date: Tue, 4 Jan 2022 17:44:26 +0800 Subject: [PATCH 11/11] Update lib/internal/modules/esm/handle_process_exit.js Co-authored-by: Antoine du Hamel --- lib/internal/modules/esm/handle_process_exit.js | 1 - 1 file changed, 1 deletion(-) diff --git a/lib/internal/modules/esm/handle_process_exit.js b/lib/internal/modules/esm/handle_process_exit.js index 8453c7d88c6473..db830900bd3154 100644 --- a/lib/internal/modules/esm/handle_process_exit.js +++ b/lib/internal/modules/esm/handle_process_exit.js @@ -3,7 +3,6 @@ // Handle a Promise from running code that potentially does Top-Level Await. // In that case, it makes sense to set the exit code to a specific non-zero // value if the main code never finishes running. -// The original purpose was https://github.com/nodejs/node/pull/34640 function handleProcessExit() { process.exitCode ??= 13; }