From 8010fb047788675efe7a4ad94010d88c73f86d5d Mon Sep 17 00:00:00 2001 From: Christian Praiss Date: Tue, 28 Jul 2026 16:06:42 +0200 Subject: [PATCH] fix(signal): Improve signal handling robustness on shutdown Refactors signal forwarding across processes and TUI interactions to ensure graceful shutdowns. In `exec.go`, we now consistently forward SIGINT/SIGTERM signals rather than escalating to a hard kill, respecting child process termination sequences (e.g., docker compose). The `tui.go` model also updates to track repeated stop attempts and utilize incremental signaling before resorting to force termination. --- exec.go | 28 ++++++++++++++-------------- tui.go | 35 +++++++++++++++++++++-------------- 2 files changed, 35 insertions(+), 28 deletions(-) diff --git a/exec.go b/exec.go index cd23c2f..dcec97d 100644 --- a/exec.go +++ b/exec.go @@ -102,13 +102,19 @@ func isTTY() bool { } // runWithSignalForwarding starts cmd and blocks until it exits, forwarding -// SIGINT/SIGTERM to it instead of letting Go's default handling kill this -// process immediately. Without this, a single Ctrl+C would kill the devops-cli -// wrapper right away while a child like `docker compose up` kept running its -// own graceful shutdown in the background, printing to the shared terminal -// after control had already returned to the shell. A second signal escalates -// to a hard kill, matching the usual "one Ctrl+C to stop gracefully, two to -// force it" shell convention. +// every SIGINT/SIGTERM we receive straight through to it instead of letting +// Go's default handling kill this process immediately. Without this, a +// single Ctrl+C would kill the devops-cli wrapper right away while a child +// like `docker compose up` kept running its own graceful shutdown in the +// background, printing to the shared terminal after control had already +// returned to the shell. +// +// Deliberately does NOT escalate to SIGKILL on a repeated signal: tools like +// `docker compose` already implement their own "first Ctrl+C graceful, second +// forces it" handling internally. If we SIGKILL the child ourselves instead +// of forwarding that second signal, we yank it out mid-shutdown before it +// can tell the engine to stop remaining containers, potentially leaving them +// running — worse than doing nothing. func runWithSignalForwarding(cmd *exec.Cmd) error { sigCh := make(chan os.Signal, 1) signal.Notify(sigCh, syscall.SIGINT, syscall.SIGTERM) @@ -120,16 +126,10 @@ func runWithSignalForwarding(cmd *exec.Cmd) error { done := make(chan struct{}) go func() { - forwarded := false for { select { case sig := <-sigCh: - if !forwarded { - forwarded = true - _ = cmd.Process.Signal(sig) - } else { - _ = cmd.Process.Kill() - } + _ = cmd.Process.Signal(sig) case <-done: return } diff --git a/tui.go b/tui.go index b1327fc..857478a 100644 --- a/tui.go +++ b/tui.go @@ -160,14 +160,15 @@ func statusFrame(p *panel, tick int) string { // ── Model ──────────────────────────────────────────────────────────────────── type model struct { - panels []*panel - focus int - width int - height int - tick int - ch chan tea.Msg - running int - stopping bool + panels []*panel + focus int + width int + height int + tick int + ch chan tea.Msg + running int + stopping bool + stopPresses int } func newModel(panels []*panel) model { @@ -244,16 +245,22 @@ func (m model) Update(msg tea.Msg) (tea.Model, tea.Cmd) { if m.running == 0 { return m, tea.Quit } - if !m.stopping { - // First press: ask running panels to stop gracefully and - // wait for them to actually exit before quitting the TUI. - m.stopping = true + m.stopping = true + m.stopPresses++ + if m.stopPresses < 3 { + // Forward the interrupt (again) and wait for panels to exit + // on their own. Tools like `docker compose` implement their + // own graceful-then-force handling on a second SIGINT, so we + // keep forwarding rather than killing them ourselves — a + // SIGKILL from us would cut a graceful shutdown off mid-way + // (e.g. containers left running) instead of letting it finish. for _, panel := range m.panels { panel.signal(os.Interrupt) } return m, nil } - // Second press: force-kill anything still running and quit now. + // Third press: something isn't responding to SIGINT. Force-kill + // as a last resort so the TUI doesn't trap the user forever. for _, panel := range m.panels { panel.signal(os.Kill) } @@ -372,7 +379,7 @@ func (m model) View() string { case m.running == 0: barText = fmt.Sprintf(" Done: %d/%d succeeded [q]uit [tab/←→/h/l]focus [↑↓/PgUp/PgDn/j/k]scroll [g/G]top/end", okCount, n) case m.stopping: - barText = fmt.Sprintf(" Stopping (%d active)… press again to force quit", m.running) + barText = fmt.Sprintf(" Stopping (%d active)… press %d more time(s) to force quit", m.running, imax(0, 3-m.stopPresses)) default: barText = fmt.Sprintf(" Running (%d active) [q]uit [tab/←→/h/l]focus [↑↓/PgUp/PgDn/j/k]scroll [g/G]top/end", m.running) }