From 3563c496c58e9cdbe43ab88b1fb4def5554a27bf Mon Sep 17 00:00:00 2001 From: kishore280 Date: Sun, 23 Aug 2026 22:56:40 +0530 Subject: [PATCH 1/3] child_process: clear timeout timer on spawn-time error too When spawn() fails at the OS level (ENOENT, EACCES, EAGAIN, EMFILE, ENFILE), the resulting ChildProcess only ever emits 'error', never 'exit'. The `timeout` option's cleanup only listened for 'exit', so the timer stayed armed for the full `timeout` duration on any spawn-time failure, holding the event loop open well after the promise/callback had already settled via 'error'. Clear the timer on 'error' as well as 'exit'. Fixes: https://github.com/nodejs/node/issues/65504 Signed-off-by: kishore280 --- lib/child_process.js | 8 +++++-- ...ld-process-spawn-timeout-clear-on-error.js | 21 +++++++++++++++++++ 2 files changed, 27 insertions(+), 2 deletions(-) create mode 100644 test/parallel/test-child-process-spawn-timeout-clear-on-error.js diff --git a/lib/child_process.js b/lib/child_process.js index 0e3e04af0d6e..d70790c3c8f2 100644 --- a/lib/child_process.js +++ b/lib/child_process.js @@ -824,12 +824,16 @@ function spawn(file, args, options) { } }, options.timeout); - child.once('exit', () => { + const clearSpawnTimeout = () => { if (timeoutId) { clearTimeout(timeoutId); timeoutId = null; } - }); + }; + + // A spawn-time failure only emits 'error', never 'exit'. + child.once('exit', clearSpawnTimeout); + child.once('error', clearSpawnTimeout); } if (options.signal) { diff --git a/test/parallel/test-child-process-spawn-timeout-clear-on-error.js b/test/parallel/test-child-process-spawn-timeout-clear-on-error.js new file mode 100644 index 000000000000..74bf40afc3d8 --- /dev/null +++ b/test/parallel/test-child-process-spawn-timeout-clear-on-error.js @@ -0,0 +1,21 @@ +'use strict'; + +// The timeout timer must clear on a spawn-time 'error', not just 'exit'. + +const common = require('../common'); +const assert = require('assert'); +const { spawn } = require('child_process'); + +const start = Date.now(); + +const cp = spawn(process.execPath, ['--version'], { + cwd: '/nonexistent/path/that/should/never/exist', + timeout: common.platformTimeout(10000), +}); + +cp.on('error', common.mustCall((err) => { + assert.strictEqual(err.code, 'ENOENT'); + assert.ok(Date.now() - start < 2000); +})); + +cp.on('exit', common.mustNotCall()); From 6ddc9483d80e7ca446135e3fceec5154e0edf5a1 Mon Sep 17 00:00:00 2001 From: kishore280 Date: Mon, 24 Aug 2026 11:27:14 +0530 Subject: [PATCH 2/3] child_process: use 'close' instead of 'exit'+'error' for timeout cleanup 'close' fires after 'exit' or 'error', so one listener is enough. The old 'error' listener also had a side effect I didn't intend: it quietly stopped Node's 'Unhandled error event' crash for anyone using timeout without their own error handler. Not this fix's job. Signed-off-by: kishore280 --- lib/child_process.js | 9 +++---- ...ld-process-spawn-timeout-clear-on-error.js | 27 ++++++++++--------- 2 files changed, 17 insertions(+), 19 deletions(-) diff --git a/lib/child_process.js b/lib/child_process.js index d70790c3c8f2..e5ea1954291c 100644 --- a/lib/child_process.js +++ b/lib/child_process.js @@ -824,16 +824,13 @@ function spawn(file, args, options) { } }, options.timeout); - const clearSpawnTimeout = () => { + // 'close' always fires after 'exit', or after 'error' on a spawn failure. + child.once('close', () => { if (timeoutId) { clearTimeout(timeoutId); timeoutId = null; } - }; - - // A spawn-time failure only emits 'error', never 'exit'. - child.once('exit', clearSpawnTimeout); - child.once('error', clearSpawnTimeout); + }); } if (options.signal) { diff --git a/test/parallel/test-child-process-spawn-timeout-clear-on-error.js b/test/parallel/test-child-process-spawn-timeout-clear-on-error.js index 74bf40afc3d8..274648f29091 100644 --- a/test/parallel/test-child-process-spawn-timeout-clear-on-error.js +++ b/test/parallel/test-child-process-spawn-timeout-clear-on-error.js @@ -1,21 +1,22 @@ 'use strict'; -// The timeout timer must clear on a spawn-time 'error', not just 'exit'. +// Measures the child's actual exit time, not just its 'error' event - the outer spawnSync timeout catches a leaked inner timer. const common = require('../common'); const assert = require('assert'); -const { spawn } = require('child_process'); +const { spawnSync } = require('child_process'); -const start = Date.now(); +const bugStallMs = common.platformTimeout(10000); +const outerTimeoutMs = common.platformTimeout(2000); -const cp = spawn(process.execPath, ['--version'], { - cwd: '/nonexistent/path/that/should/never/exist', - timeout: common.platformTimeout(10000), -}); +const child = spawnSync(process.execPath, ['-e', ` + const { spawn } = require('child_process'); + const cp = spawn(process.execPath, ['--version'], { + cwd: '/nonexistent/path/that/should/never/exist', + timeout: ${bugStallMs}, + }); + cp.on('error', () => {}); +`], { timeout: outerTimeoutMs }); -cp.on('error', common.mustCall((err) => { - assert.strictEqual(err.code, 'ENOENT'); - assert.ok(Date.now() - start < 2000); -})); - -cp.on('exit', common.mustNotCall()); +assert.strictEqual(child.signal, null); +assert.strictEqual(child.status, 0); From 7c6b37f492545c46408d813554e56aae91490e09 Mon Sep 17 00:00:00 2001 From: kishore280 Date: Mon, 24 Aug 2026 21:47:03 +0530 Subject: [PATCH 3/3] test: use spawnSyncAndExitWithoutError, fix overlong comment line Signed-off-by: kishore280 --- ...test-child-process-spawn-timeout-clear-on-error.js | 11 ++++------- 1 file changed, 4 insertions(+), 7 deletions(-) diff --git a/test/parallel/test-child-process-spawn-timeout-clear-on-error.js b/test/parallel/test-child-process-spawn-timeout-clear-on-error.js index 274648f29091..f674e4ed55f0 100644 --- a/test/parallel/test-child-process-spawn-timeout-clear-on-error.js +++ b/test/parallel/test-child-process-spawn-timeout-clear-on-error.js @@ -1,15 +1,15 @@ 'use strict'; -// Measures the child's actual exit time, not just its 'error' event - the outer spawnSync timeout catches a leaked inner timer. +// Measures the child's actual exit time, not just its 'error' event. +// The outer spawnSync timeout catches a leaked inner timer. const common = require('../common'); -const assert = require('assert'); -const { spawnSync } = require('child_process'); +const { spawnSyncAndExitWithoutError } = require('../common/child_process'); const bugStallMs = common.platformTimeout(10000); const outerTimeoutMs = common.platformTimeout(2000); -const child = spawnSync(process.execPath, ['-e', ` +spawnSyncAndExitWithoutError(process.execPath, ['-e', ` const { spawn } = require('child_process'); const cp = spawn(process.execPath, ['--version'], { cwd: '/nonexistent/path/that/should/never/exist', @@ -17,6 +17,3 @@ const child = spawnSync(process.execPath, ['-e', ` }); cp.on('error', () => {}); `], { timeout: outerTimeoutMs }); - -assert.strictEqual(child.signal, null); -assert.strictEqual(child.status, 0);