Repository navigation
Conversation
Closing the terminal of `gemdb app.py` ended the bash wrapper but not topaz, which ignores SIGHUP: it carried on running the program with its session (and a web app's port), out of sight. A program that kept printing was not ended by its writes to the dead terminal either, and its finally blocks never ran. Measured on 4.0.0.a4 by closing a pty under it. The wrapper's EXIT trap, which bash runs on a hangup, now sends a topaz that is still running SIGINT, what Ctrl-C sends: the program's finally blocks run and topaz logs out and exits, within a quarter of a second in the measurement. SIGTERM follows if it has not gone in ten seconds. The temporary files are removed only after topaz has gone, since the driver writes the status file as it unwinds. The integration test now closes a real pty, as a terminal tab does, and requires topaz to end on its own with the finally block run. Against the previous wrapper it fails in 15 s rather than timing out: it waits on expect's exit, not its pipes, which a surviving topaz holds open. Fixes #136. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This branch has not been deployed
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
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.
Fixes #136.
The problem
Closing the terminal of
gemdb app.py, or of Run Python File in GemDB, ended the bash wrapper but not topaz. Topaz carried on running the program out of sight, holding a session and, for a web app, its port. Measured on 4.0.0.a4 by closing a pty under it, as a terminal tab closes:finallynever ran in either case.finally.The fix
The wrapper's
EXITtrap from #132 already runs on a hangup. It now checks whether topaz is still running (only when the run didn't finish, so a normal run does nothing extra):KeyboardInterrupt, the program'sfinallyblocks run, and topaz logs out and exits. In the measurement that took under 250 ms, for a quiet program and a printing one.Topaz is found with
pgrep -P $$ -x topaz. On a system withoutpgrep, nothing is found and the run behaves as before.Tests
Integration. The hangup test from Keep topaz's own output off the terminal during a gemdb run #132 used to end topaz itself. It now closes a real pty through expect and requires:
finallyblock to have run,$TMPDIRto be empty.Against
main's wrapper it fails in 15 s; with the fix it passes in about 1.2 s, and passed three runs in a row. The test resolves on expect'sexitevent rather than on its output closing, because a surviving topaz holds expect's pipes open and the test would otherwise time out instead of failing.Unit. Checks that the trap is set before any temporary file exists, that it only looks for topaz while topaz is running, and that it sends SIGINT.
Suites. Lint, format, both typechecks, the unit suite (901) and the CLI integration file (23) pass on darwin-arm64. A full integration run under heavy machine load (load average around 11) failed:
breakpoint.test.tsandspaceFull.test.tstimed out in setup, and twodebugConsole.test.tstests failed. Rerun on their own afterwards, all three files passed (45 tests). None of them runs the wrapper.🤖 Generated with Claude Code