Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Revert Signal Handling Regression #188

Merged
merged 1 commit into from
Jan 3, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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])
})