Skip to content

Commit 33d4e6c

Browse files
marcopiracciniaduh95
authored andcommitted
child_process: add subprocess.timedOut
When the timeout option of spawn(), exec(), execFile() or fork() expires, the child is sent the kill signal and exits with that signal. Nothing told the caller that the timeout was the reason. Set subprocess.timedOut to true when the kill signal is sent because of the timeout, and add the same flag to the error passed to the exec() and execFile() callback. Fixes: #51561 Refs: #51608 Signed-off-by: marcopiraccini <marco.piraccini@gmail.com> PR-URL: #66639 Reviewed-By: Paolo Insogna <paolo@cowtech.it>
1 parent 2e27c83 commit 33d4e6c

7 files changed

Lines changed: 139 additions & 8 deletions

‎doc/api/child_process.md‎

Lines changed: 29 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -233,7 +233,8 @@ If a `callback` function is provided, it is called with the arguments
233233
`error` will be an instance of [`Error`][]. The `error.code` property will be
234234
the exit code of the process. By convention, any exit code other than `0`
235235
indicates an error. `error.signal` will be the signal that terminated the
236-
process.
236+
process. `error.timedOut` will be `true` if the process was killed because
237+
the `timeout` option expired.
237238

238239
The `stdout` and `stderr` arguments passed to the callback will contain the
239240
stdout and stderr output of the child process. By default, Node.js will decode
@@ -2316,6 +2317,32 @@ subprocess.stdout.on('data', (data) => {
23162317
The `subprocess.stdout` property can be `null` or `undefined`
23172318
if the child process could not be successfully spawned.
23182319
2320+
### `subprocess.timedOut`
2321+
2322+
<!-- YAML
2323+
added: REPLACEME
2324+
-->
2325+
2326+
* Type: {boolean} Set to `true` when the child process is killed because the
2327+
`timeout` option expired.
2328+
2329+
The `subprocess.timedOut` property indicates whether the `timeout` option of
2330+
[`child_process.spawn()`][], [`child_process.exec()`][],
2331+
[`child_process.execFile()`][] or [`child_process.fork()`][] expired and the
2332+
child process was sent the `killSignal` as a result. Like
2333+
[`subprocess.killed`][], it does not indicate that the child process has
2334+
terminated yet.
2335+
2336+
```js
2337+
const { spawn } = require('node:child_process');
2338+
2339+
const subprocess = spawn('sleep', ['10'], { timeout: 100 });
2340+
2341+
subprocess.on('exit', (code, signal) => {
2342+
console.log(subprocess.timedOut); // true
2343+
});
2344+
```
2345+
23192346
### `subprocess.unref()`
23202347
23212348
<!-- YAML
@@ -2431,6 +2458,7 @@ or [`child_process.fork()`][].
24312458
[`subprocess.disconnect()`]: #subprocessdisconnect
24322459
[`subprocess.exitCode`]: #subprocessexitcode
24332460
[`subprocess.kill()`]: #subprocesskillsignal
2461+
[`subprocess.killed`]: #subprocesskilled
24342462
[`subprocess.send()`]: #subprocesssendmessage-sendhandle-options-callback
24352463
[`subprocess.signalCode`]: #subprocesssignalcode
24362464
[`subprocess.stderr`]: #subprocessstderr

‎lib/child_process.js‎

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -426,6 +426,7 @@ function execFile(file, args, options, callback) {
426426
});
427427

428428
ex.cmd = cmd;
429+
ex.timedOut = child.timedOut;
429430
callback(ex, stdout, stderr);
430431
}
431432

@@ -450,16 +451,19 @@ function execFile(file, args, options, callback) {
450451

451452
killed = true;
452453
try {
453-
child.kill(options.killSignal);
454+
return child.kill(options.killSignal);
454455
} catch (e) {
455456
ex = e;
456457
exithandler();
457458
}
459+
return false;
458460
}
459461

460462
if (options.timeout > 0) {
461463
timeoutId = setTimeout(function delayedKill() {
462-
kill();
464+
// The child may have exited already, with a descendant keeping its
465+
// stdio open: kill() reports whether the signal was actually sent.
466+
child.timedOut = kill();
463467
timeoutId = null;
464468
}, options.timeout);
465469
}
@@ -895,7 +899,9 @@ function spawn(file, args, options) {
895899
let timeoutId = setTimeout(() => {
896900
if (timeoutId) {
897901
try {
898-
child.kill(killSignal);
902+
// The child may have exited already, with a descendant keeping its
903+
// stdio open: kill() reports whether the signal was actually sent.
904+
child.timedOut = child.kill(killSignal);
899905
} catch (err) {
900906
child.emit('error', err);
901907
}

‎lib/internal/child_process.js‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -291,6 +291,7 @@ function ChildProcess() {
291291
this.signalCode = null;
292292
this.exitCode = null;
293293
this.killed = false;
294+
this.timedOut = false;
294295
this.spawnfile = null;
295296

296297
this._handle = new Process();

‎test/parallel/test-child-process-exec-timeout-expire.js‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,7 @@ cp.exec(cmd, {
2929

3030
let sigterm = 'SIGTERM';
3131
assert.strictEqual(err.killed, true);
32+
assert.strictEqual(err.timedOut, true);
3233
// TODO OpenBSD returns a null signal and 143 for code
3334
if (common.isOpenBSD) {
3435
assert.strictEqual(err.code, 143);

‎test/parallel/test-child-process-exec-timeout-kill.js‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,7 @@ if (process.argv[2] === 'child') {
2121
const [cmd, opts] = common.escapePOSIXShell`"${process.execPath}" "${__filename}" child`;
2222

2323
// Test with a different kill signal.
24-
cp.exec(cmd, {
24+
const child = cp.exec(cmd, {
2525
...opts,
2626
timeout: kExpiringParentTimer,
2727
killSignal: 'SIGKILL'
@@ -30,6 +30,8 @@ cp.exec(cmd, {
3030
console.log('[stderr]', stderr.trim());
3131

3232
assert.strictEqual(err.killed, true);
33+
assert.strictEqual(err.timedOut, true);
34+
assert.strictEqual(child.timedOut, true);
3335
assert.strictEqual(err.code, null);
3436
assert.strictEqual(err.signal, 'SIGKILL');
3537
assert.strictEqual(err.cmd, cmd);

‎test/parallel/test-child-process-exec-timeout-not-expired.js‎

Lines changed: 39 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -22,14 +22,52 @@ if (process.argv[2] === 'child') {
2222
return;
2323
}
2424

25+
// The property is also present on errors that are not caused by the timeout.
26+
cp.execFile('this-command-does-not-exist', { timeout: 2 ** 30 },
27+
common.mustCall((err) => {
28+
assert.strictEqual(err.code, 'ENOENT');
29+
assert.strictEqual(err.timedOut, false);
30+
}));
31+
32+
// The timeout expires after the child exited, while a descendant still keeps
33+
// its stdio open: the signal is not sent, so timedOut stays false. The child
34+
// prints the pid of the descendant and the test ends it once the timer has
35+
// fired, so the test does not depend on how fast the child starts.
36+
{
37+
const timeout = common.platformTimeout(5000);
38+
const child = cp.execFile(process.execPath, ['-e', `
39+
const { spawn } = require('child_process');
40+
// Detached: on Windows a non-detached descendant is killed with the job
41+
// object of the child when the child exits.
42+
const child = spawn(process.execPath,
43+
['-e', 'setTimeout(() => {}, ${common.platformTimeout(30000)})'],
44+
{ stdio: 'inherit', detached: true });
45+
child.unref();
46+
process.stdout.write(String(child.pid));
47+
`], { timeout }, common.mustSucceed(() => {
48+
assert.strictEqual(child.exitCode, 0);
49+
assert.strictEqual(child.timedOut, false);
50+
}));
51+
const pid = new Promise((resolve) => {
52+
child.stdout.once('data', (chunk) => resolve(Number(String(chunk))));
53+
});
54+
child.on('exit', common.mustCall((code) => {
55+
assert.strictEqual(code, 0);
56+
// The timeout of the child started before this timer, so it fires first.
57+
// The descendant is ended after that, which closes the stdio of the child.
58+
setTimeout(async () => process.kill(await pid), timeout);
59+
}));
60+
}
61+
2562
const [cmd, opts] = common.escapePOSIXShell`"${process.execPath}" "${__filename}" child`;
2663

27-
cp.exec(cmd, {
64+
const child = cp.exec(cmd, {
2865
...opts,
2966
timeout: kTimeoutNotSupposedToExpire,
3067
}, common.mustSucceed((stdout, stderr) => {
3168
assert.strictEqual(stdout.trim(), 'child stdout');
3269
assert.strictEqual(stderr.trim(), 'child stderr');
70+
assert.strictEqual(child.timedOut, false);
3371
}));
3472

3573
cleanupStaleProcess(__filename);

‎test/parallel/test-child-process-spawn-timeout-kill-signal.js‎

Lines changed: 57 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
'use strict';
22

3-
const { mustCall } = require('../common');
3+
const common = require('../common');
4+
const { mustCall } = common;
45
const assert = require('assert');
56
const fixtures = require('../common/fixtures');
67
const { spawn } = require('child_process');
@@ -12,7 +13,61 @@ const aliveForeverFile = 'child-process-stay-alive-forever.js';
1213
const cp = spawn(process.execPath, [fixtures.path(aliveForeverFile)], {
1314
timeout: 5,
1415
});
15-
cp.on('exit', mustCall((code, ks) => assert.strictEqual(ks, 'SIGTERM')));
16+
assert.strictEqual(cp.timedOut, false);
17+
cp.on('exit', mustCall((code, ks) => {
18+
assert.strictEqual(ks, 'SIGTERM');
19+
assert.strictEqual(cp.timedOut, true);
20+
}));
21+
}
22+
23+
{
24+
// Verify timedOut stays false when the child exits before the timeout
25+
const cp = spawn(process.execPath, ['-e', ''], { timeout: 2 ** 30 });
26+
cp.on('exit', mustCall((code) => {
27+
assert.strictEqual(code, 0);
28+
assert.strictEqual(cp.timedOut, false);
29+
}));
30+
}
31+
32+
{
33+
// Verify timedOut stays false when the child exits in time but a descendant
34+
// keeps its stdio open past the timeout, so that the timer fires after exit.
35+
// The child prints the pid of the descendant and the test ends it once the
36+
// timer has fired, so the test does not depend on how fast the child starts.
37+
const timeout = common.platformTimeout(5000);
38+
const cp = spawn(process.execPath, ['-e', `
39+
const { spawn } = require('child_process');
40+
// Detached: on Windows a non-detached descendant is killed with the job
41+
// object of the child when the child exits.
42+
const child = spawn(process.execPath,
43+
['-e', 'setTimeout(() => {}, ${common.platformTimeout(30000)})'],
44+
{ stdio: 'inherit', detached: true });
45+
child.unref();
46+
process.stdout.write(String(child.pid));
47+
`], { timeout });
48+
const pid = new Promise((resolve) => {
49+
cp.stdout.once('data', (chunk) => resolve(Number(String(chunk))));
50+
});
51+
cp.on('exit', mustCall((code) => {
52+
assert.strictEqual(code, 0);
53+
assert.strictEqual(cp.timedOut, false);
54+
// The timeout of the child started before this timer, so it fires first.
55+
// The descendant is ended after that, which closes the stdio of the child.
56+
setTimeout(async () => process.kill(await pid), timeout);
57+
}));
58+
cp.on('close', mustCall(() => assert.strictEqual(cp.timedOut, false)));
59+
}
60+
61+
{
62+
// Verify timedOut stays false when the child is killed by the caller
63+
const cp = spawn(process.execPath, [fixtures.path(aliveForeverFile)], {
64+
timeout: 2 ** 30,
65+
});
66+
cp.on('spawn', mustCall(() => cp.kill()));
67+
cp.on('exit', mustCall(() => {
68+
assert.strictEqual(cp.killed, true);
69+
assert.strictEqual(cp.timedOut, false);
70+
}));
1671
}
1772

1873
{

0 commit comments

Comments
 (0)