FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

child_process: allow promisified exec to be cancel · nodejs/node@da98c9f · GitHub

/ node Public

Commit da98c9f

Browse files
metcoder95
authored and
MylesBorins
committed
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 <duhamelantoine1995@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com>
1 parent 1cbb74d commit da98c9f

4 files changed

Lines changed: 160 additions & 27 deletions

‎lib/child_process.js‎

Lines changed: 34 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -43,7 +43,6 @@ let debug = require('internal/util/debuglog').debuglog(
4343
debug = fn;
4444
}
4545
);
46-
const { AbortController } = require('internal/abort_controller');
4746
const { Buffer } = require('buffer');
4847
const { Pipe, constants: PipeConstants } = internalBinding('pipe_wrap');
4948

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

153-
return spawnWithSignal(options.execPath, args, options);
152+
return spawn(options.execPath, args, options);
154153
}
155154

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

314-
// Validate signal, if present
315-
validateAbortSignal(options.signal, 'options.signal');
316-
317313
options.killSignal = sanitizeKillSignal(options.killSignal);
318314

319315
const child = spawn(file, args, {
320316
cwd: options.cwd,
321317
env: options.env,
322318
gid: options.gid,
323-
uid: options.uid,
324319
shell: options.shell,
320+
signal: options.signal,
321+
uid: options.uid,
325322
windowsHide: !!options.windowsHide,
326323
windowsVerbatimArguments: !!options.windowsVerbatimArguments
327324
});
@@ -425,28 +422,12 @@ function execFile(file /* , args, options, callback */) {
425422
}
426423
}
427424

428-
function abortHandler() {
429-
if (!ex)
430-
ex = new AbortError();
431-
process.nextTick(() => kill());
432-
}
433-
434425
if (options.timeout > 0) {
435426
timeoutId = setTimeout(function delayedKill() {
436427
kill();
437428
timeoutId = null;
438429
}, options.timeout);
439430
}
440-
if (options.signal) {
441-
if (options.signal.aborted) {
442-
process.nextTick(abortHandler);
443-
} else {
444-
const childController = new AbortController();
445-
options.signal.addEventListener('abort', abortHandler,
446-
{ signal: childController.signal });
447-
child.once('close', () => childController.abort());
448-
}
449-
}
450431

451432
if (child.stdout) {
452433
if (encoding)
@@ -661,8 +642,31 @@ function normalizeSpawnArguments(file, args, options) {
661642
*/
662643
function spawn(file, args, options) {
663644
const child = new ChildProcess();
664-
665645
options = normalizeSpawnArguments(file, args, options);
646+
647+
if (options.signal) {
648+
const signal = options.signal;
649+
// Validate signal, if present
650+
validateAbortSignal(signal, 'options.signal');
651+
652+
// Do nothing and throw if already aborted
653+
if (signal.aborted) {
654+
onAbortListener();
655+
} else {
656+
signal.addEventListener('abort', onAbortListener, { once: true });
657+
child.once('close',
658+
() => signal.removeEventListener('abort', onAbortListener));
659+
}
660+
661+
function onAbortListener() {
662+
process.nextTick(() => {
663+
child?.kill?.(options.killSignal);
664+
665+
child.emit('error', new AbortError());
666+
});
667+
}
668+
}
669+
666670
debug('spawn', options);
667671
child.spawn(options);
668672

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

873882
if (options && options.signal) {
874883
// Validate signal, if present
875884
validateAbortSignal(options.signal, 'options.signal');
876885
function kill() {
877886
if (child._handle) {
878-
child.kill('SIGTERM');
887+
child._handle.kill(options.killSignal || 'SIGTERM');
879888
child.emit('error', new AbortError());
880889
}
881890
}

‎test/parallel/test-child-process-exec-abortcontroller-promisified.js‎

Lines changed: 52 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,52 @@
1+
// Flags: --experimental-abortcontroller
2+
'use strict';
3+
const common = require('../common');
4+
const assert = require('assert');
5+
const exec = require('child_process').exec;
6+
const { promisify } = require('util');
7+
8+
let pwdcommand, dir;
9+
const execPromisifed = promisify(exec);
10+
const invalidArgTypeError = {
11+
code: 'ERR_INVALID_ARG_TYPE',
12+
name: 'TypeError'
13+
};
14+
15+
16+
if (common.isWindows) {
17+
pwdcommand = 'echo %cd%';
18+
dir = 'c:\\windows';
19+
} else {
20+
pwdcommand = 'pwd';
21+
dir = '/dev';
22+
}
23+
24+
25+
{
26+
const ac = new AbortController();
27+
const signal = ac.signal;
28+
const promise = execPromisifed(pwdcommand, { cwd: dir, signal });
29+
assert.rejects(promise, /AbortError/).then(common.mustCall());
30+
ac.abort();
31+
}
32+
33+
{
34+
assert.throws(() => {
35+
execPromisifed(pwdcommand, { cwd: dir, signal: {} });
36+
}, invalidArgTypeError);
37+
}
38+
39+
{
40+
function signal() {}
41+
assert.throws(() => {
42+
execPromisifed(pwdcommand, { cwd: dir, signal });
43+
}, invalidArgTypeError);
44+
}
45+
46+
{
47+
const ac = new AbortController();
48+
const signal = (ac.abort(), ac.signal);
49+
const promise = execPromisifed(pwdcommand, { cwd: dir, signal });
50+
51+
assert.rejects(promise, /AbortError/).then(common.mustCall());
52+
}

‎test/parallel/test-child-process-execFile-promisified-abortController.js‎

Lines changed: 59 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,59 @@
1+
// Flags: --experimental-abortcontroller
2+
'use strict';
3+
4+
const common = require('../common');
5+
const assert = require('assert');
6+
const { promisify } = require('util');
7+
const execFile = require('child_process').execFile;
8+
const fixtures = require('../common/fixtures');
9+
10+
const echoFixture = fixtures.path('echo.js');
11+
const promisified = promisify(execFile);
12+
const invalidArgTypeError = {
13+
code: 'ERR_INVALID_ARG_TYPE',
14+
name: 'TypeError'
15+
};
16+
17+
{
18+
// Verify that the signal option works properly
19+
const ac = new AbortController();
20+
const signal = ac.signal;
21+
const promise = promisified(process.execPath, [echoFixture, 0], { signal });
22+
23+
ac.abort();
24+
25+
assert.rejects(
26+
promise,
27+
{ name: 'AbortError' }
28+
).then(common.mustCall());
29+
}
30+
31+
{
32+
// Verify that the signal option works properly when already aborted
33+
const ac = new AbortController();
34+
const { signal } = ac;
35+
ac.abort();
36+
37+
assert.rejects(
38+
promisified(process.execPath, [echoFixture, 0], { signal }),
39+
{ name: 'AbortError' }
40+
).then(common.mustCall());
41+
}
42+
43+
{
44+
// Verify that if something different than Abortcontroller.signal
45+
// is passed, ERR_INVALID_ARG_TYPE is thrown
46+
const signal = {};
47+
assert.throws(() => {
48+
promisified(process.execPath, [echoFixture, 0], { signal });
49+
}, invalidArgTypeError);
50+
}
51+
52+
{
53+
// Verify that if something different than Abortcontroller.signal
54+
// is passed, ERR_INVALID_ARG_TYPE is thrown
55+
const signal = 'world!';
56+
assert.throws(() => {
57+
promisified(process.execPath, [echoFixture, 0], { signal });
58+
}, invalidArgTypeError);
59+
}

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

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

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

7285
{

0 commit comments

Comments
 (0)
Back | FazBrowse Home | New Git URL