Skip to content

Commit 7b78c67

Browse files
metcoder95targos
authored andcommitted
child_process: allow promisified exec to be cancel
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]>
1 parent d76400a commit 7b78c67

4 files changed

+158
-26
lines changed

lib/child_process.js

+34-24
Original file line numberDiff line numberDiff line change
@@ -140,7 +140,7 @@ function fork(modulePath /* , args, options */) {
140140
options.execPath = options.execPath || process.execPath;
141141
options.shell = false;
142142

143-
return spawnWithSignal(options.execPath, args, options);
143+
return spawn(options.execPath, args, options);
144144
}
145145

146146
function _forkChild(fd, serializationMode) {
@@ -254,17 +254,15 @@ function execFile(file /* , args, options, callback */) {
254254
// Validate maxBuffer, if present.
255255
validateMaxBuffer(options.maxBuffer);
256256

257-
// Validate signal, if present
258-
validateAbortSignal(options.signal, 'options.signal');
259-
260257
options.killSignal = sanitizeKillSignal(options.killSignal);
261258

262259
const child = spawn(file, args, {
263260
cwd: options.cwd,
264261
env: options.env,
265262
gid: options.gid,
266-
uid: options.uid,
267263
shell: options.shell,
264+
signal: options.signal,
265+
uid: options.uid,
268266
windowsHide: !!options.windowsHide,
269267
windowsVerbatimArguments: !!options.windowsVerbatimArguments
270268
});
@@ -368,28 +366,12 @@ function execFile(file /* , args, options, callback */) {
368366
}
369367
}
370368

371-
function abortHandler() {
372-
if (!ex)
373-
ex = new AbortError();
374-
process.nextTick(() => kill());
375-
}
376-
377369
if (options.timeout > 0) {
378370
timeoutId = setTimeout(function delayedKill() {
379371
kill();
380372
timeoutId = null;
381373
}, options.timeout);
382374
}
383-
if (options.signal) {
384-
if (options.signal.aborted) {
385-
process.nextTick(abortHandler);
386-
} else {
387-
const childController = new AbortController();
388-
options.signal.addEventListener('abort', abortHandler,
389-
{ signal: childController.signal });
390-
child.once('close', () => childController.abort());
391-
}
392-
}
393375

394376
if (child.stdout) {
395377
if (encoding)
@@ -611,8 +593,31 @@ function normalizeSpawnArguments(file, args, options) {
611593

612594
function spawn(file, args, options) {
613595
const child = new ChildProcess();
614-
615596
options = normalizeSpawnArguments(file, args, options);
597+
598+
if (options.signal) {
599+
const signal = options.signal;
600+
// Validate signal, if present
601+
validateAbortSignal(signal, 'options.signal');
602+
603+
// Do nothing and throw if already aborted
604+
if (signal.aborted) {
605+
onAbortListener();
606+
} else {
607+
signal.addEventListener('abort', onAbortListener, { once: true });
608+
child.once('close',
609+
() => signal.removeEventListener('abort', onAbortListener));
610+
}
611+
612+
function onAbortListener() {
613+
process.nextTick(() => {
614+
child?.kill?.(options.killSignal);
615+
616+
child.emit('error', new AbortError());
617+
});
618+
}
619+
}
620+
616621
debug('spawn', options);
617622
child.spawn(options);
618623

@@ -752,14 +757,19 @@ function sanitizeKillSignal(killSignal) {
752757
// This level of indirection is here because the other child_process methods
753758
// call spawn internally but should use different cancellation logic.
754759
function spawnWithSignal(file, args, options) {
755-
const child = spawn(file, args, options);
760+
// Remove signal from options to spawn
761+
// to avoid double emitting of AbortError
762+
const opts = options && typeof options === 'object' && ('signal' in options) ?
763+
{ ...options, signal: undefined } :
764+
options;
765+
const child = spawn(file, args, opts);
756766

757767
if (options && options.signal) {
758768
// Validate signal, if present
759769
validateAbortSignal(options.signal, 'options.signal');
760770
function kill() {
761771
if (child._handle) {
762-
child.kill('SIGTERM');
772+
child._handle.kill(options.killSignal || 'SIGTERM');
763773
child.emit('error', new AbortError());
764774
}
765775
}
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,51 @@
1+
'use strict';
2+
const common = require('../common');
3+
const assert = require('assert');
4+
const exec = require('child_process').exec;
5+
const { promisify } = require('util');
6+
7+
let pwdcommand, dir;
8+
const execPromisifed = promisify(exec);
9+
const invalidArgTypeError = {
10+
code: 'ERR_INVALID_ARG_TYPE',
11+
name: 'TypeError'
12+
};
13+
14+
15+
if (common.isWindows) {
16+
pwdcommand = 'echo %cd%';
17+
dir = 'c:\\windows';
18+
} else {
19+
pwdcommand = 'pwd';
20+
dir = '/dev';
21+
}
22+
23+
24+
{
25+
const ac = new AbortController();
26+
const signal = ac.signal;
27+
const promise = execPromisifed(pwdcommand, { cwd: dir, signal });
28+
assert.rejects(promise, /AbortError/).then(common.mustCall());
29+
ac.abort();
30+
}
31+
32+
{
33+
assert.throws(() => {
34+
execPromisifed(pwdcommand, { cwd: dir, signal: {} });
35+
}, invalidArgTypeError);
36+
}
37+
38+
{
39+
function signal() {}
40+
assert.throws(() => {
41+
execPromisifed(pwdcommand, { cwd: dir, signal });
42+
}, invalidArgTypeError);
43+
}
44+
45+
{
46+
const ac = new AbortController();
47+
const signal = (ac.abort(), ac.signal);
48+
const promise = execPromisifed(pwdcommand, { cwd: dir, signal });
49+
50+
assert.rejects(promise, /AbortError/).then(common.mustCall());
51+
}
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,58 @@
1+
'use strict';
2+
3+
const common = require('../common');
4+
const assert = require('assert');
5+
const { promisify } = require('util');
6+
const execFile = require('child_process').execFile;
7+
const fixtures = require('../common/fixtures');
8+
9+
const echoFixture = fixtures.path('echo.js');
10+
const promisified = promisify(execFile);
11+
const invalidArgTypeError = {
12+
code: 'ERR_INVALID_ARG_TYPE',
13+
name: 'TypeError'
14+
};
15+
16+
{
17+
// Verify that the signal option works properly
18+
const ac = new AbortController();
19+
const signal = ac.signal;
20+
const promise = promisified(process.execPath, [echoFixture, 0], { signal });
21+
22+
ac.abort();
23+
24+
assert.rejects(
25+
promise,
26+
{ name: 'AbortError' }
27+
).then(common.mustCall());
28+
}
29+
30+
{
31+
// Verify that the signal option works properly when already aborted
32+
const ac = new AbortController();
33+
const { signal } = ac;
34+
ac.abort();
35+
36+
assert.rejects(
37+
promisified(process.execPath, [echoFixture, 0], { signal }),
38+
{ name: 'AbortError' }
39+
).then(common.mustCall());
40+
}
41+
42+
{
43+
// Verify that if something different than Abortcontroller.signal
44+
// is passed, ERR_INVALID_ARG_TYPE is thrown
45+
const signal = {};
46+
assert.throws(() => {
47+
promisified(process.execPath, [echoFixture, 0], { signal });
48+
}, invalidArgTypeError);
49+
}
50+
51+
{
52+
// Verify that if something different than Abortcontroller.signal
53+
// is passed, ERR_INVALID_ARG_TYPE is thrown
54+
const signal = 'world!';
55+
assert.throws(() => {
56+
promisified(process.execPath, [echoFixture, 0], { signal });
57+
}, invalidArgTypeError);
58+
}

test/parallel/test-child-process-execfile.js

+15-2
Original file line numberDiff line numberDiff line change
@@ -62,10 +62,23 @@ const execOpts = { encoding: 'utf8', shell: true };
6262
execFile(process.execPath, [echoFixture, 0], { signal }, check);
6363
};
6464

65-
test();
66-
ac.abort();
6765
// Verify that it still works the same way now that the signal is aborted.
6866
test();
67+
ac.abort();
68+
}
69+
70+
{
71+
// Verify that does not spawn a child if already aborted
72+
const ac = new AbortController();
73+
const { signal } = ac;
74+
ac.abort();
75+
76+
const check = common.mustCall((err) => {
77+
assert.strictEqual(err.code, 'ABORT_ERR');
78+
assert.strictEqual(err.name, 'AbortError');
79+
assert.strictEqual(err.signal, undefined);
80+
});
81+
execFile(process.execPath, [echoFixture, 0], { signal }, check);
6982
}
7083

7184
{

0 commit comments

Comments
 (0)