Skip to content

Commit

Permalink
child_process: add timeout and killSignal to spawn and fork
Browse files Browse the repository at this point in the history
Add support for timeout and killSignal to spawn and fork.

Fixes: #27639
  • Loading branch information
Nitzan Uziely committed Feb 6, 2021
1 parent d6e9446 commit 8d45929
Show file tree
Hide file tree
Showing 6 changed files with 256 additions and 2 deletions.
14 changes: 14 additions & 0 deletions doc/api/child_process.md
Original file line number Diff line number Diff line change
Expand Up @@ -351,6 +351,9 @@ controller.abort();
<!-- YAML
added: v0.5.0
changes:
- version: REPLACEME
pr-url: xxx
description: killSignal and timeout was added.
- version: v15.6.0
pr-url: https://github.com/nodejs/node/pull/36603
description: AbortSignal support was added.
Expand Down Expand Up @@ -394,6 +397,10 @@ 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`.
* `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}

The `child_process.fork()` method is a special case of
Expand Down Expand Up @@ -431,6 +438,9 @@ The `signal` option works exactly the same way it does in
<!-- YAML
added: v0.1.90
changes:
- version: REPLACEME
pr-url: xxx
description: killSignal and timeout were added.
- version: v15.5.0
pr-url: https://github.com/nodejs/node/pull/36432
description: AbortSignal support was added.
Expand Down Expand Up @@ -477,6 +487,10 @@ changes:
* `windowsHide` {boolean} Hide the subprocess console window that would
normally be created on Windows systems. **Default:** `false`.
* `signal` {AbortSignal} allows aborting the execFile using an AbortSignal.
* `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}

Expand Down
42 changes: 41 additions & 1 deletion lib/child_process.js
Original file line number Diff line number Diff line change
Expand Up @@ -141,7 +141,26 @@ function fork(modulePath /* , args, options */) {
options.execPath = options.execPath || process.execPath;
options.shell = false;

return spawn(options.execPath, args, options);
validateTimeout(options.timeout);
if (options.killSignal) {
options.killSignal = sanitizeKillSignal(options.killSignal);
}
const child = spawn(options.execPath, args, options);

if (options.timeout > 0) {
let timeoutId = setTimeout(() => {
child.kill(options.killSignal);
timeoutId = null;
}, options.timeout);

child.once('close', () => {
if (timeoutId) {
clearTimeout(timeoutId);
timeoutId = null;
}
});
}
return child;
}

function _forkChild(fd, serializationMode) {
Expand Down Expand Up @@ -758,6 +777,12 @@ function spawnWithSignal(file, args, options) {
const opts = options && typeof options === 'object' && ('signal' in options) ?
{ ...options, signal: undefined } :
options;

// Validate the timeout, if present.
validateTimeout(options?.timeout);
if (options?.killSignal) {
options.killSignal = sanitizeKillSignal(options.killSignal);
}
const child = spawn(file, args, opts);

if (options && options.signal) {
Expand All @@ -777,6 +802,21 @@ function spawnWithSignal(file, args, options) {
child.once('close', remove);
}
}

if (options?.timeout > 0) {
let timeoutId = setTimeout(() => {
child.kill(options.killSignal);
timeoutId = null;
}, options.timeout);

child.once('close', () => {
if (timeoutId) {
clearTimeout(timeoutId);
timeoutId = null;
}
});
}

return child;
}
module.exports = {
Expand Down
17 changes: 16 additions & 1 deletion test/parallel/test-child-process-fork-abort-signal.js
Original file line number Diff line number Diff line change
Expand Up @@ -26,8 +26,23 @@ const { fork } = require('child_process');
const cp = fork(fixtures.path('child-process-stay-alive-forever.js'), {
signal
});
cp.on('exit', mustCall());
cp.on('exit', mustCall((code, ks) => strictEqual(ks, 'SIGTERM')));
cp.on('error', mustCall((err) => {
strictEqual(err.name, 'AbortError');
}));
}

{
// Test correct signal sent
const ac = new AbortController();
const { signal } = ac;
const cp = fork(fixtures.path('child-process-stay-alive-forever.js'), {
signal,
killSignal: 'SIGKILL',
});
cp.on('exit', mustCall((code, ks) => strictEqual(ks, 'SIGKILL')));
cp.on('error', mustCall((err) => {
strictEqual(err.name, 'AbortError');
}));
process.nextTick(() => ac.abort());
}
69 changes: 69 additions & 0 deletions test/parallel/test-child-process-fork-timeout-kill-signal.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,69 @@
'use strict';

const { mustCall } = require('../common');
const { strictEqual, ok, throws } = require('assert');
const fixtures = require('../common/fixtures');
const { fork } = require('child_process');
const { getEventListeners } = require('events');

{
// Verify default signal + closes after at least 4 ms.
let closed = false;
setTimeout(() => ok(!closed), 4);
const cp = fork(fixtures.path('child-process-stay-alive-forever.js'), {
timeout: 5,
});
cp.on('close', mustCall(() => closed = true));
cp.on('exit', mustCall((code, ks) => strictEqual(ks, 'SIGTERM')));
}

{
// Verify correct signal + closes after at least 4 ms.
let closed = false;
setTimeout(() => ok(!closed), 4);
const cp = fork(fixtures.path('child-process-stay-alive-forever.js'), {
timeout: 5,
killSignal: 'SIGKILL',
});
cp.on('close', mustCall(() => closed = true));
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('close', mustCall(() => {
strictEqual(getEventListeners(signal, 'abort').length, 0);
}));
}

{
// Verify no error happens when abort is called timeout
const controller = new AbortController();
const { signal } = controller;

const cp = fork(fixtures.path('child-process-stay-alive-forever.js'), {
timeout: 100,
signal,
});
cp.on('error', mustCall());
setTimeout(() => controller.abort(), 1);
}
46 changes: 46 additions & 0 deletions test/parallel/test-child-process-spawn-controller.js
Original file line number Diff line number Diff line change
Expand Up @@ -39,3 +39,49 @@ const cp = require('child_process');
assert.strictEqual(e.name, 'AbortError');
}));
}

{
// Verify that passing kill signal works.
const controller = new AbortController();
const { signal } = controller;

const echo = cp.spawn('echo', ['fun'], {
encoding: 'utf8',
shell: true,
signal,
killSignal: 'SIGKILL',
});

echo.on('close', common.mustCall((code, killSignal) => {
assert.strictEqual(killSignal, 'SIGKILL');
}));

echo.on('error', common.mustCall((e) => {
assert.strictEqual(e.name, 'AbortError');
}));

controller.abort();
}

{
// Verify that passing a different kill signal works.
const controller = new AbortController();
const { signal } = controller;

const echo = cp.spawn('echo', ['fun'], {
encoding: 'utf8',
shell: true,
signal,
killSignal: 'SIGTERM',
});

echo.on('close', common.mustCall((code, killSignal) => {
assert.strictEqual(killSignal, 'SIGTERM');
}));

echo.on('error', common.mustCall((e) => {
assert.strictEqual(e.name, 'AbortError');
}));

controller.abort();
}
70 changes: 70 additions & 0 deletions test/parallel/test-child-process-spawn-timeout-kill-signal.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,70 @@
'use strict';

const { mustCall } = require('../common');
const { strictEqual, ok, 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 after at least 4 ms.
let closed = false;
setTimeout(() => ok(!closed), 4);
const cp = spawn(process.execPath, [fixtures.path(aliveForeverFile)], {
timeout: 5,
});
cp.on('close', mustCall(() => closed = true));
cp.on('exit', mustCall((code, ks) => strictEqual(ks, 'SIGTERM')));
}

{
// Verify correct signal + closes after at least 4 ms.
let closed = false;
setTimeout(() => ok(!closed), 4);
const cp = spawn(process.execPath, [fixtures.path(aliveForeverFile)], {
timeout: 5,
killSignal: 'SIGKILL',
});
cp.on('close', mustCall(() => closed = true));
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 signal = new EventTarget();
signal.aborted = false;

const cp = spawn(process.execPath, [fixtures.path(aliveForeverFile)], {
timeout: 6,
signal,
});
strictEqual(getEventListeners(signal, 'abort').length, 1);
cp.on('close', mustCall(() => {
strictEqual(getEventListeners(signal, 'abort').length, 0);
}));
}

{
// Verify no error happens when abort is called timeout
const controller = new AbortController();
const { signal } = controller;

const cp = spawn(process.execPath, [fixtures.path(aliveForeverFile)], {
timeout: 100,
signal,
});
cp.on('error', mustCall());
setTimeout(() => controller.abort(), 1);
}

0 comments on commit 8d45929

Please sign in to comment.