fix(daemon): stop the remote browser on SIGTERM#475
Open
yangshu2087 wants to merge 1 commit into
Open
Conversation
The daemon only caught SIGINT (KeyboardInterrupt); SIGTERM terminated the process before the finally block, so stop_remote() never ran and the remote browser leaked (and kept billing). admin.py escalates to os.kill(pid, SIGTERM) when the "shutdown" IPC can't be delivered, and container runtimes send SIGTERM on stop, so this path is hit in practice. Register an asyncio SIGTERM handler that sets the existing stop Event — the same graceful path as the "shutdown" command — so main() unwinds cleanly and the finally block runs stop_remote(). Registered before start() (the CDP handshake can take seconds) with the stop Event moved to __init__. POSIX-only: add_signal_handler raises NotImplementedError on Windows, where SIGTERM can't be caught anyway. Adds a unit test asserting main() registers the handler and that the stop Event drives a graceful return. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
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.
Problem
The daemon only handled
SIGINT(KeyboardInterrupt). OnSIGTERMthe process was terminated before thefinallyblock ran, sostop_remote()never fired and the remote browser leaked (and kept billing).This path is hit in practice:
admin.pyescalates toos.kill(pid, signal.SIGTERM)when the graceful"shutdown"IPC can't be delivered (hung/unreachable daemon).SIGTERMon stop.Fix
Register an asyncio
SIGTERMhandler that sets the existingstopEvent — the same graceful path as the"shutdown"command — somain()unwinds cleanly and thefinallyblock runsstop_remote().start(), becauseget_ws_url()/ the CDP handshake can take seconds and a signal during that window must still trigger cleanup. ThestopEvent is moved to__init__so it exists in time.add_signal_handlerraisesNotImplementedErroron Windows, whereSIGTERMcan't be caught anyway — guarded.Test
Adds
test_main_registers_sigterm_handler_that_triggers_graceful_stop: assertsmain()registers the handler (via the publicloop.remove_signal_handler, which returnsTrueiff one was set) and that setting the stop Event drives a graceful return. No real signal is delivered (a regression would otherwise kill the test runner). Skipped on Windows.Full suite:
115 passed.🤖 Generated with Claude Code
Summary by cubic
Ensure the daemon stops the remote browser on SIGTERM to avoid leaks and surprise billing. A SIGTERM now triggers the same graceful shutdown path so stop_remote() always runs.
Written for commit cfd7e18. Summary will update on new commits.