Skip to content

Commit

Permalink
child_process: allow promisified exec to be cancel
Browse files Browse the repository at this point in the history
Using new AbortController, add support for promisified
exec to be cancelled.

PR-URL: #34249
Reviewed-By: Antoine du Hamel <[email protected]>
Reviewed-By: James M Snell <[email protected]>
  • Loading branch information
metcoder95 authored and MylesBorins committed Aug 31, 2021
1 parent 1cbb74d commit da98c9f
Show file tree
Hide file tree
Showing 4 changed files with 160 additions and 27 deletions.
59 changes: 34 additions & 25 deletions lib/child_process.js
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,6 @@ let debug = require('internal/util/debuglog').debuglog(
debug = fn;
}
);
const { AbortController } = require('internal/abort_controller');
const { Buffer } = require('buffer');
const { Pipe, constants: PipeConstants } = internalBinding('pipe_wrap');

Expand Down Expand Up @@ -150,7 +149,7 @@ function fork(modulePath /* , args, options */) {
options.execPath = options.execPath || process.execPath;
options.shell = false;

return spawnWithSignal(options.execPath, args, options);
return spawn(options.execPath, args, options);
}

function _forkChild(fd, serializationMode) {
Expand Down Expand Up @@ -311,17 +310,15 @@ function execFile(file /* , args, options, callback */) {
// Validate maxBuffer, if present.
validateMaxBuffer(options.maxBuffer);

// Validate signal, if present
validateAbortSignal(options.signal, 'options.signal');

options.killSignal = sanitizeKillSignal(options.killSignal);

const child = spawn(file, args, {
cwd: options.cwd,
env: options.env,
gid: options.gid,
uid: options.uid,
shell: options.shell,
signal: options.signal,
uid: options.uid,
windowsHide: !!options.windowsHide,
windowsVerbatimArguments: !!options.windowsVerbatimArguments
});
Expand Down Expand Up @@ -425,28 +422,12 @@ function execFile(file /* , args, options, callback */) {
}
}

function abortHandler() {
if (!ex)
ex = new AbortError();
process.nextTick(() => kill());
}

if (options.timeout > 0) {
timeoutId = setTimeout(function delayedKill() {
kill();
timeoutId = null;
}, options.timeout);
}
if (options.signal) {
if (options.signal.aborted) {
process.nextTick(abortHandler);
} else {
const childController = new AbortController();
options.signal.addEventListener('abort', abortHandler,
{ signal: childController.signal });
child.once('close', () => childController.abort());
}
}

if (child.stdout) {
if (encoding)
Expand Down Expand Up @@ -661,8 +642,31 @@ function normalizeSpawnArguments(file, args, options) {
*/
function spawn(file, args, options) {
const child = new ChildProcess();

options = normalizeSpawnArguments(file, args, options);

if (options.signal) {
const signal = options.signal;
// Validate signal, if present
validateAbortSignal(signal, 'options.signal');

// Do nothing and throw if already aborted
if (signal.aborted) {
onAbortListener();
} else {
signal.addEventListener('abort', onAbortListener, { once: true });
child.once('close',
() => signal.removeEventListener('abort', onAbortListener));
}

function onAbortListener() {
process.nextTick(() => {
child?.kill?.(options.killSignal);

child.emit('error', new AbortError());
});
}
}

debug('spawn', options);
child.spawn(options);

Expand Down Expand Up @@ -868,14 +872,19 @@ function sanitizeKillSignal(killSignal) {
// This level of indirection is here because the other child_process methods
// call spawn internally but should use different cancellation logic.
function spawnWithSignal(file, args, options) {
const child = spawn(file, args, options);
// Remove signal from options to spawn
// to avoid double emitting of AbortError
const opts = options && typeof options === 'object' && ('signal' in options) ?
{ ...options, signal: undefined } :
options;
const child = spawn(file, args, opts);

if (options && options.signal) {
// Validate signal, if present
validateAbortSignal(options.signal, 'options.signal');
function kill() {
if (child._handle) {
child.kill('SIGTERM');
child._handle.kill(options.killSignal || 'SIGTERM');
child.emit('error', new AbortError());
}
}
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,52 @@
// Flags: --experimental-abortcontroller
'use strict';
const common = require('../common');
const assert = require('assert');
const exec = require('child_process').exec;
const { promisify } = require('util');

let pwdcommand, dir;
const execPromisifed = promisify(exec);
const invalidArgTypeError = {
code: 'ERR_INVALID_ARG_TYPE',
name: 'TypeError'
};


if (common.isWindows) {
pwdcommand = 'echo %cd%';
dir = 'c:\\windows';
} else {
pwdcommand = 'pwd';
dir = '/dev';
}


{
const ac = new AbortController();
const signal = ac.signal;
const promise = execPromisifed(pwdcommand, { cwd: dir, signal });
assert.rejects(promise, /AbortError/).then(common.mustCall());
ac.abort();
}

{
assert.throws(() => {
execPromisifed(pwdcommand, { cwd: dir, signal: {} });
}, invalidArgTypeError);
}

{
function signal() {}
assert.throws(() => {
execPromisifed(pwdcommand, { cwd: dir, signal });
}, invalidArgTypeError);
}

{
const ac = new AbortController();
const signal = (ac.abort(), ac.signal);
const promise = execPromisifed(pwdcommand, { cwd: dir, signal });

assert.rejects(promise, /AbortError/).then(common.mustCall());
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,59 @@
// Flags: --experimental-abortcontroller
'use strict';

const common = require('../common');
const assert = require('assert');
const { promisify } = require('util');
const execFile = require('child_process').execFile;
const fixtures = require('../common/fixtures');

const echoFixture = fixtures.path('echo.js');
const promisified = promisify(execFile);
const invalidArgTypeError = {
code: 'ERR_INVALID_ARG_TYPE',
name: 'TypeError'
};

{
// Verify that the signal option works properly
const ac = new AbortController();
const signal = ac.signal;
const promise = promisified(process.execPath, [echoFixture, 0], { signal });

ac.abort();

assert.rejects(
promise,
{ name: 'AbortError' }
).then(common.mustCall());
}

{
// Verify that the signal option works properly when already aborted
const ac = new AbortController();
const { signal } = ac;
ac.abort();

assert.rejects(
promisified(process.execPath, [echoFixture, 0], { signal }),
{ name: 'AbortError' }
).then(common.mustCall());
}

{
// Verify that if something different than Abortcontroller.signal
// is passed, ERR_INVALID_ARG_TYPE is thrown
const signal = {};
assert.throws(() => {
promisified(process.execPath, [echoFixture, 0], { signal });
}, invalidArgTypeError);
}

{
// Verify that if something different than Abortcontroller.signal
// is passed, ERR_INVALID_ARG_TYPE is thrown
const signal = 'world!';
assert.throws(() => {
promisified(process.execPath, [echoFixture, 0], { signal });
}, invalidArgTypeError);
}
17 changes: 15 additions & 2 deletions test/parallel/test-child-process-execfile.js
Original file line number Diff line number Diff line change
Expand Up @@ -63,10 +63,23 @@ const execOpts = { encoding: 'utf8', shell: true };
execFile(process.execPath, [echoFixture, 0], { signal }, check);
};

test();
ac.abort();
// Verify that it still works the same way now that the signal is aborted.
test();
ac.abort();
}

{
// Verify that does not spawn a child if already aborted
const ac = new AbortController();
const { signal } = ac;
ac.abort();

const check = common.mustCall((err) => {
assert.strictEqual(err.code, 'ABORT_ERR');
assert.strictEqual(err.name, 'AbortError');
assert.strictEqual(err.signal, undefined);
});
execFile(process.execPath, [echoFixture, 0], { signal }, check);
}

{
Expand Down

0 comments on commit da98c9f

Please sign in to comment.