Repository navigation
Conversation
Closing the terminal that hosts `ringer run` sends SIGHUP. Ringer handled only SIGINT and SIGTERM, so Python's default killed it at once: workers were orphaned and the run state was never finished. SIGHUP now goes through the same one-shot graceful shutdown. An inherited SIG_IGN (nohup) is respected. Because the terminal is gone, stdout/stderr fds that were ttys are pointed at /dev/null first; otherwise cleanup completes but the interpreter exits 120 on the final flush (EIO). Pipes and files keep their output, and shutdown is requested even if the redirect fails. Tests: SIGHUP over a pipe (summary preserved), a real pty hangup with the controller closed before the signal, and SIGHUP under nohup (run and worker keep going until SIGTERM). The existing SIGTERM test now shares the helper. Co-Authored-By: Claude Opus 5.5 <[email protected]> Claude-Session: https://claude.ai/code/session_01VuxjLwN47bGDGAKEv5Nfz6
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closing the terminal window that hosted
ringer runkilled in-progress stages outright. The run state stayed unfinished and worker processes were left orphaned, so a task that had done nothing wrong showed up as an error. We hit it when the operator's terminal app closed mid-stage.The cause: the terminal sends SIGHUP, and ringer only installs handlers for SIGINT and SIGTERM. At Python's default disposition SIGHUP terminates the process immediately — none of the existing one-shot cleanup (worker group termination, state flush) runs.
What this changes
request_shutdown(), so the existing "second signal does not cancel cleanup" behavior applies to it too.SIG_IGN, no handler is installed, sonohup ./ringer.py run …keeps running through a hangup exactly as before.EIO. Without handling, cleanup does complete but the interpreter exits 120 on the final flush instead of 130. So the stdout/stderr fds that were ttys when the handlers were registered are pointed at/dev/nullbefore shutdown. Pipes and files keep their output (the summary still arrives), and shutdown is requested even if that redirect fails.hasattr; loop signal handlers stay suppressed as before.Proof
Three tests, plus the existing SIGTERM test refactored into a shared helper:
test_sighup_cleans_up_active_worker_and_finishes_state: SIGHUP over a pipe → exit 130, worker gone, state finished, andYour results:still in the output.test_terminal_hangup_cleans_up_even_though_output_is_gone: ringer runs on a pty, the controller is closed before the signal (a real window close) → exit 130, worker gone, state finished.test_sighup_is_ignored_under_nohup: started with SIGHUP ignored, a SIGHUP leaves both the run and the worker alive; SIGTERM then shuts down cleanly.The first two fail on current
main(-1and120instead of130) and pass with the change. The full suite passes locally on macOS (python3 -m unittest discover -s tests, 287 tests,RINGER_NO_SELF_UPDATE=1).Small and single-purpose:
ringer.py+27/−2 (mostly comments and the guarded redirect), the rest is tests.🤖 Generated with Claude Code
https://claude.ai/code/session_01VuxjLwN47bGDGAKEv5Nfz6