From 8eba0aeea05d4076654bf19e7ee06fa23dc7e324 Mon Sep 17 00:00:00 2001 From: Nitzan Uziely Date: Sat, 20 Mar 2021 01:02:18 +0200 Subject: [PATCH 1/2] child_process: cleanup AbortSignal duplication cleanup AbortSignal child_process code duplication --- lib/child_process.js | 67 ++++++++++++-------------------------------- 1 file changed, 18 insertions(+), 49 deletions(-) diff --git a/lib/child_process.js b/lib/child_process.js index 18996f8a5643ce..e82d6dc2baba26 100644 --- a/lib/child_process.js +++ b/lib/child_process.js @@ -605,23 +605,6 @@ function spawn(file, args, options) { const killSignal = sanitizeKillSignal(options.killSignal); const child = new ChildProcess(); - if (options.signal) { - const signal = options.signal; - if (signal.aborted) { - onAbortListener(); - } else { - signal.addEventListener('abort', onAbortListener, { once: true }); - child.once('exit', - () => signal.removeEventListener('abort', onAbortListener)); - } - - function onAbortListener() { - process.nextTick(() => { - abortChildProcess(child, killSignal); - }); - } - } - debug('spawn', options); child.spawn(options); @@ -645,6 +628,23 @@ function spawn(file, args, options) { }); } + if (options.signal) { + const signal = options.signal; + if (signal.aborted) { + process.nextTick(() => { + onAbortListener(); + }); + } else { + signal.addEventListener('abort', onAbortListener, { once: true }); + child.once('exit', + () => signal.removeEventListener('abort', onAbortListener)); + } + + function onAbortListener() { + abortChildProcess(child, killSignal); + } + } + return child; } @@ -778,37 +778,6 @@ 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) { - // 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; - - if (options?.signal) { - // Validate signal, if present - validateAbortSignal(options.signal, 'options.signal'); - } - const child = spawn(file, args, opts); - - if (options?.signal) { - const killSignal = sanitizeKillSignal(options.killSignal); - - function kill() { - abortChildProcess(child, killSignal); - } - if (options.signal.aborted) { - process.nextTick(kill); - } else { - options.signal.addEventListener('abort', kill, { once: true }); - const remove = () => options.signal.removeEventListener('abort', kill); - child.once('exit', remove); - } - } - return child; -} module.exports = { _forkChild, ChildProcess, @@ -817,6 +786,6 @@ module.exports = { execFileSync, execSync, fork, - spawn: spawnWithSignal, + spawn, spawnSync }; From 474f0658771e2543f9fb9b949ace73cde3abcac6 Mon Sep 17 00:00:00 2001 From: Nitzan Uziely Date: Sat, 20 Mar 2021 12:10:18 +0200 Subject: [PATCH 2/2] fixup! child_process: cleanup AbortSignal duplication --- lib/child_process.js | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/lib/child_process.js b/lib/child_process.js index e82d6dc2baba26..da2f9628ce0808 100644 --- a/lib/child_process.js +++ b/lib/child_process.js @@ -631,9 +631,7 @@ function spawn(file, args, options) { if (options.signal) { const signal = options.signal; if (signal.aborted) { - process.nextTick(() => { - onAbortListener(); - }); + process.nextTick(onAbortListener); } else { signal.addEventListener('abort', onAbortListener, { once: true }); child.once('exit',