Skip to content

Commit

Permalink
fix: restore signals forwarding to child processes
Browse files Browse the repository at this point in the history
Reverting the changes introduced in
commit 545f3be, as it inadvertently
disrupted proper signals support
  • Loading branch information
wiktor-obrebski committed Dec 16, 2023
1 parent 8a0443b commit d1ec4b0
Show file tree
Hide file tree
Showing 2 changed files with 27 additions and 5 deletions.
11 changes: 6 additions & 5 deletions lib/signal-manager.js
Original file line number Diff line number Diff line change
@@ -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',
Expand All @@ -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)
Expand Down
21 changes: 21 additions & 0 deletions test/signal-manager.js
Original file line number Diff line number Diff line change
Expand Up @@ -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])
})

0 comments on commit d1ec4b0

Please sign in to comment.