From d8fb5c2f1c907f09e2763046c436a236b70cdea5 Mon Sep 17 00:00:00 2001
From: Nitzan Uziely <nitzan@testim.io>
Date: Sun, 7 Feb 2021 01:39:54 +0200
Subject: [PATCH] child_process: add timeout to spawn and fork
Add support for timeout to spawn and fork.
Fixes: https://github.com/nodejs/node/issues/27639
PR-URL: https://github.com/nodejs/node/pull/37256
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
---
doc/api/child_process.md | 18 +++++--
lib/child_process.js | 29 +++++++++--
...-child-process-fork-timeout-kill-signal.js | 50 +++++++++++++++++++
...child-process-spawn-timeout-kill-signal.js | 50 +++++++++++++++++++
4 files changed, 138 insertions(+), 9 deletions(-)
create mode 100644 test/parallel/test-child-process-fork-timeout-kill-signal.js
create mode 100644 test/parallel/test-child-process-spawn-timeout-kill-signal.js
diff --git a/doc/api/child_process.md b/doc/api/child_process.md
index 677f3a5c190554..77467cc6573064 100644
--- a/doc/api/child_process.md
+++ b/doc/api/child_process.md
@@ -374,6 +374,9 @@ controller.abort();
<!-- YAML
added: v0.5.0
changes:
+ - version: REPLACEME
+ pr-url: https://github.com/nodejs/node/pull/37256
+ description: timeout was added.
- version: v15.11.0
pr-url: https://github.com/nodejs/node/pull/37325
description: killSignal for AbortSignal was added.
@@ -410,8 +413,8 @@ changes:
See [Advanced serialization][] for more details. **Default:** `'json'`.
* `signal` {AbortSignal} Allows closing the child process using an
AbortSignal.
- * `killSignal` {string} The signal value to be used when the spawned
- process will be killed by the abort signal. **Default:** `'SIGTERM'`.
+ * `killSignal` {string|integer} The signal value to be used when the spawned
+ process will be killed by timeout or abort signal. **Default:** `'SIGTERM'`.
* `silent` {boolean} If `true`, stdin, stdout, and stderr of the child will be
piped to the parent, otherwise they will be inherited from the parent, see
the `'pipe'` and `'inherit'` options for [`child_process.spawn()`][]'s
@@ -423,6 +426,8 @@ changes:
* `uid` {number} Sets the user identity of the process (see setuid(2)).
* `windowsVerbatimArguments` {boolean} No quoting or escaping of arguments is
done on Windows. Ignored on Unix. **Default:** `false`.
+ * `timeout` {number} In milliseconds the maximum amount of time the process
+ is allowed to run. **Default:** `undefined`.
* Returns: {ChildProcess}
The `child_process.fork()` method is a special case of
@@ -478,6 +483,9 @@ if (process.argv[2] === 'child') {
<!-- YAML
added: v0.1.90
changes:
+ - version: REPLACEME
+ pr-url: https://github.com/nodejs/node/pull/37256
+ description: timeout was added.
- version: v15.11.0
pr-url: https://github.com/nodejs/node/pull/37325
description: killSignal for AbortSignal was added.
@@ -528,8 +536,10 @@ changes:
normally be created on Windows systems. **Default:** `false`.
* `signal` {AbortSignal} allows aborting the child process using an
AbortSignal.
- * `killSignal` {string} The signal value to be used when the spawned
- process will be killed by the abort signal. **Default:** `'SIGTERM'`.
+ * `timeout` {number} In milliseconds the maximum amount of time the process
+ is allowed to run. **Default:** `undefined`.
+ * `killSignal` {string|integer} The signal value to be used when the spawned
+ process will be killed by timeout or abort signal. **Default:** `'SIGTERM'`.
* Returns: {ChildProcess}
diff --git a/lib/child_process.js b/lib/child_process.js
index 57b5eaa71b1bb5..18996f8a5643ce 100644
--- a/lib/child_process.js
+++ b/lib/child_process.js
@@ -599,15 +599,14 @@ function abortChildProcess(child, killSignal) {
function spawn(file, args, options) {
- const child = new ChildProcess();
options = normalizeSpawnArguments(file, args, options);
+ validateTimeout(options.timeout, 'options.timeout');
+ validateAbortSignal(options.signal, 'options.signal');
+ const killSignal = sanitizeKillSignal(options.killSignal);
+ const child = new ChildProcess();
if (options.signal) {
const signal = options.signal;
- // Validate signal, if present
- validateAbortSignal(signal, 'options.signal');
- const killSignal = sanitizeKillSignal(options.killSignal);
- // Do nothing and throw if already aborted
if (signal.aborted) {
onAbortListener();
} else {
@@ -626,6 +625,26 @@ function spawn(file, args, options) {
debug('spawn', options);
child.spawn(options);
+ if (options.timeout > 0) {
+ let timeoutId = setTimeout(() => {
+ if (timeoutId) {
+ try {
+ child.kill(killSignal);
+ } catch (err) {
+ child.emit('error', err);
+ }
+ timeoutId = null;
+ }
+ }, options.timeout);
+
+ child.once('exit', () => {
+ if (timeoutId) {
+ clearTimeout(timeoutId);
+ timeoutId = null;
+ }
+ });
+ }
+
return child;
}
diff --git a/test/parallel/test-child-process-fork-timeout-kill-signal.js b/test/parallel/test-child-process-fork-timeout-kill-signal.js
new file mode 100644
index 00000000000000..ef08d4b12ab94a
--- /dev/null
+++ b/test/parallel/test-child-process-fork-timeout-kill-signal.js
@@ -0,0 +1,50 @@
+'use strict';
+
+const { mustCall } = require('../common');
+const { strictEqual, throws } = require('assert');
+const fixtures = require('../common/fixtures');
+const { fork } = require('child_process');
+const { getEventListeners } = require('events');
+
+{
+ // Verify default signal
+ const cp = fork(fixtures.path('child-process-stay-alive-forever.js'), {
+ timeout: 5,
+ });
+ cp.on('exit', mustCall((code, ks) => strictEqual(ks, 'SIGTERM')));
+}
+
+{
+ // Verify correct signal + closes after at least 4 ms.
+ const cp = fork(fixtures.path('child-process-stay-alive-forever.js'), {
+ timeout: 5,
+ killSignal: 'SIGKILL',
+ });
+ cp.on('exit', mustCall((code, ks) => strictEqual(ks, 'SIGKILL')));
+}
+
+{
+ // Verify timeout verification
+ throws(() => fork(fixtures.path('child-process-stay-alive-forever.js'), {
+ timeout: 'badValue',
+ }), /ERR_OUT_OF_RANGE/);
+
+ throws(() => fork(fixtures.path('child-process-stay-alive-forever.js'), {
+ timeout: {},
+ }), /ERR_OUT_OF_RANGE/);
+}
+
+{
+ // Verify abort signal gets unregistered
+ const signal = new EventTarget();
+ signal.aborted = false;
+
+ const cp = fork(fixtures.path('child-process-stay-alive-forever.js'), {
+ timeout: 6,
+ signal,
+ });
+ strictEqual(getEventListeners(signal, 'abort').length, 1);
+ cp.on('exit', mustCall(() => {
+ strictEqual(getEventListeners(signal, 'abort').length, 0);
+ }));
+}
diff --git a/test/parallel/test-child-process-spawn-timeout-kill-signal.js b/test/parallel/test-child-process-spawn-timeout-kill-signal.js
new file mode 100644
index 00000000000000..59a974d0e0b448
--- /dev/null
+++ b/test/parallel/test-child-process-spawn-timeout-kill-signal.js
@@ -0,0 +1,50 @@
+'use strict';
+
+const { mustCall } = require('../common');
+const { strictEqual, throws } = require('assert');
+const fixtures = require('../common/fixtures');
+const { spawn } = require('child_process');
+const { getEventListeners } = require('events');
+
+const aliveForeverFile = 'child-process-stay-alive-forever.js';
+{
+ // Verify default signal + closes
+ const cp = spawn(process.execPath, [fixtures.path(aliveForeverFile)], {
+ timeout: 5,
+ });
+ cp.on('exit', mustCall((code, ks) => strictEqual(ks, 'SIGTERM')));
+}
+
+{
+ // Verify SIGKILL signal + closes
+ const cp = spawn(process.execPath, [fixtures.path(aliveForeverFile)], {
+ timeout: 6,
+ killSignal: 'SIGKILL',
+ });
+ cp.on('exit', mustCall((code, ks) => strictEqual(ks, 'SIGKILL')));
+}
+
+{
+ // Verify timeout verification
+ throws(() => spawn(process.execPath, [fixtures.path(aliveForeverFile)], {
+ timeout: 'badValue',
+ }), /ERR_OUT_OF_RANGE/);
+
+ throws(() => spawn(process.execPath, [fixtures.path(aliveForeverFile)], {
+ timeout: {},
+ }), /ERR_OUT_OF_RANGE/);
+}
+
+{
+ // Verify abort signal gets unregistered
+ const controller = new AbortController();
+ const { signal } = controller;
+ const cp = spawn(process.execPath, [fixtures.path(aliveForeverFile)], {
+ timeout: 6,
+ signal,
+ });
+ strictEqual(getEventListeners(signal, 'abort').length, 1);
+ cp.on('exit', mustCall(() => {
+ strictEqual(getEventListeners(signal, 'abort').length, 0);
+ }));
+}