Skip to content

Commit

Permalink
esm: make process.exit() default to exit code 0
Browse files Browse the repository at this point in the history
Due to a bug in top-level await implementation, it used to default to
exit code 13.

PR-URL: #41388
Fixes: #40808
Reviewed-By: Antoine du Hamel <[email protected]>
Reviewed-By: Guy Bedford <[email protected]>
  • Loading branch information
MoonBall authored and danielleadams committed Feb 1, 2022
1 parent 121199f commit 3cfe856
Show file tree
Hide file tree
Showing 7 changed files with 51 additions and 9 deletions.
12 changes: 12 additions & 0 deletions lib/internal/modules/esm/handle_process_exit.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
'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.
function handleProcessExit() {
process.exitCode ??= 13;
}

module.exports = {
handleProcessExit,
};
14 changes: 5 additions & 9 deletions lib/internal/modules/run_main.js
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -53,18 +56,11 @@ 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.
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);
}
}

Expand Down
6 changes: 6 additions & 0 deletions lib/internal/process/per_thread.js
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down Expand Up @@ -175,6 +179,8 @@ function wrapProcessMethods(binding) {
memoryUsage.rss = rss;

function exit(code) {
process.off('exit', handleProcessExit);

if (code || code === 0)
process.exitCode = code;

Expand Down
18 changes: 18 additions & 0 deletions test/es-module/test-esm-tla-unfinished.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -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, '', '']);
}
1 change: 1 addition & 0 deletions test/fixtures/es-modules/tla/process-exit.mjs
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
process.exit();
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
import { Worker, isMainThread } from 'worker_threads';

if (isMainThread) {
new Worker(new URL(import.meta.url));
await new Promise(() => {});
} else {
process.exit();
}
1 change: 1 addition & 0 deletions test/parallel/test-bootstrap-modules.js
Original file line number Diff line number Diff line change
Expand Up @@ -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',
Expand Down

0 comments on commit 3cfe856

Please sign in to comment.