From d1ec4b065419cf5faf1658ea0e1671ffe5b8bad4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Wiktor=20Obr=C4=99bski?= Date: Sat, 16 Dec 2023 21:01:16 +0100 Subject: [PATCH] fix: restore signals forwarding to child processes Reverting the changes introduced in commit 545f3be94d412941537ad0011717933d48cb58cf, as it inadvertently disrupted proper signals support --- lib/signal-manager.js | 11 ++++++----- test/signal-manager.js | 21 +++++++++++++++++++++ 2 files changed, 27 insertions(+), 5 deletions(-) diff --git a/lib/signal-manager.js b/lib/signal-manager.js index efc00b4..a099a4a 100644 --- a/lib/signal-manager.js +++ b/lib/signal-manager.js @@ -1,9 +1,6 @@ const runningProcs = new Set() let handlersInstalled = false -// NOTE: these signals aren't actually forwarded anywhere. they're trapped and -// ignored until all child processes have exited. in our next breaking change -// we should rename this const forwardedSignals = [ 'SIGINT', 'SIGTERM', @@ -12,8 +9,12 @@ const forwardedSignals = [ // no-op, this is so receiving the signal doesn't cause us to exit immediately // instead, we exit after all children have exited when we re-send the signal // to ourselves. see the catch handler at the bottom of run-script-pkg.js -// istanbul ignore next - this function does nothing -const handleSignal = () => {} +const handleSignal = signal => { + for (const proc of runningProcs) { + proc.kill(signal) + } +} + const setupListeners = () => { for (const signal of forwardedSignals) { process.on(signal, handleSignal) diff --git a/test/signal-manager.js b/test/signal-manager.js index 7a0544f..afc0ab3 100644 --- a/test/signal-manager.js +++ b/test/signal-manager.js @@ -44,3 +44,24 @@ test('adds only one handler for each signal, removes handlers when children have t.end() }) + +test('forwards signals to child process', t => { + const proc = new EventEmitter() + proc.kill = (signal) => { + t.equal(signal, signalManager.forwardedSignals[0], 'child receives correct signal') + proc.emit('exit', 0) + for (const forwarded of signalManager.forwardedSignals) { + t.equal( + process.listeners(forwarded).includes(signalManager.handleSignal), + false, 'listener has been removed') + } + t.end() + } + + signalManager.add(proc) + // passing the signal name here is necessary to fake the effects of actually + // receiving the signal per nodejs documentation signal handlers receive the + // name of the signal as their first parameter + // https://nodejs.org/api/process.html#process_signal_events + process.emit(signalManager.forwardedSignals[0], signalManager.forwardedSignals[0]) +})