feat(process): escalate SIGTERM→SIGKILL on host-pid termination after grace (#15008) - #50489
Merged
Conversation
Contributor
🔎 Lint report:
|
| Rule | Count |
|---|---|
invalid-argument-type |
4 |
not-subscriptable |
3 |
unresolved-attribute |
1 |
First entries
tests/tools/test_process_registry.py:1565: [unresolved-attribute] unresolved-attribute: Attribute `readline` is not defined on `None` in union `IO[Any] | None`
tools/process_registry.py:513: [not-subscriptable] not-subscriptable: Cannot subscript object of type `None` with no `__getitem__` method
tools/process_registry.py:513: [not-subscriptable] not-subscriptable: Cannot subscript object of type `float` with no `__getitem__` method
tools/process_registry.py:513: [invalid-argument-type] invalid-argument-type: Method `__getitem__` of type `Overload[(i: SupportsIndex, /) -> Unknown, (s: slice[SupportsIndex | None, SupportsIndex | None, SupportsIndex | None], /) -> list[Unknown]]` cannot be called with key of type `Literal["daemon_term_grace_seconds"]` on object of type `list[Unknown]`
tools/process_registry.py:514: [invalid-argument-type] invalid-argument-type: Argument to constructor `float.__new__` is incorrect: Expected `str | Buffer | SupportsFloat | SupportsIndex`, found `Unknown | int | str | ... omitted 15 union elements`
tools/process_registry.py:513: [not-subscriptable] not-subscriptable: Cannot subscript object of type `int` with no `__getitem__` method
tools/process_registry.py:513: [invalid-argument-type] invalid-argument-type: Method `__getitem__` of type `bound method str.__getitem__(key: SupportsIndex | slice[SupportsIndex | None, SupportsIndex | None, SupportsIndex | None], /) -> str` cannot be called with key of type `Literal["daemon_term_grace_seconds"]` on object of type `str`
tools/process_registry.py:513: [invalid-argument-type] invalid-argument-type: Method `__getitem__` of type `Overload[(i: SupportsIndex, /) -> str, (s: slice[SupportsIndex | None, SupportsIndex | None, SupportsIndex | None], /) -> list[str]]` cannot be called with key of type `Literal["daemon_term_grace_seconds"]` on object of type `list[str]`
✅ Fixed issues: none
Unchanged: 5975 pre-existing issues carried over.
Diagnostics are surfaced as warnings — this check never fails the build.
alt-glitch
added
type/bug
…r grace A daemon that ignores or stalls in its SIGTERM handler currently survives the process-registry reap and leaks until reboot (observed as agent-browser daemons accumulating to EMFILE on long-running gateways). _terminate_host_pid now snapshots the tree, SIGTERMs it, waits a bounded grace window (terminal.daemon_term_grace_seconds, default 2.0s, 0 disables), then SIGKILLs any survivor. The recycled-PID identity guard still gates the whole path, so escalation never reaches a stranger; Windows is unchanged (taskkill /F is already a hard kill). Config lives in config.yaml (terminal.daemon_term_grace_seconds), NOT an env var, per the .env-secrets-only policy. Implements the SIGKILL-escalation idea from @tkwong's #15008, reworked onto the current _terminate_host_pid tree-kill path (the original predated it) and config-gated instead of env-var-gated. Co-authored-by: Benjamin Wong <tkwong@inspiresynergy.com>
…cs survivors Live testing against a real SIGTERM-ignoring process TREE (parent + children, the agent-browser daemon + renderer shape) revealed psutil.wait_procs's gone/alive partition mis-handles a parent/child tree: it reaps via Process.wait() and could mark targets gone/alive inconsistently across the tree, leaving survivors un-killed (flaky — sometimes the parent lived, sometimes a child). Replace it with: sleep out the grace window, then directly re-probe every captured target (_proc_alive, treating zombies as dead) and SIGKILL any that's still running. Add a multi-child-tree regression test. 6/6 escalation tests green across repeated runs; the real-tree E2E now kills the full tree 6/6 runs.
teknium1
force-pushed
the
feat/daemon-sigkill-escalation
branch
from
June 22, 2026 01:10
dd387bf to
4494c9c
Compare
3 tasks
1 task
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
2 participants
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.
Summary
A host daemon that ignores or stalls in its SIGTERM handler no longer leaks indefinitely —
_terminate_host_pidnow escalates to SIGKILL after a bounded, config-gated grace window. Implements the escalation idea from @tkwong's #15008, reworked onto the current tree-kill path and config-gated (not env-var-gated).Root cause:
_terminate_host_pidSIGTERMed the process tree and returned. A daemon that traps/stalls on SIGTERM (someagent-browserdaemons do) survives the reap and leaks until reboot — observed as daemons accumulating to EMFILE on long-running gateways.Changes
tools/process_registry.py:_terminate_host_pidsnapshots the tree, SIGTERMs it (children first), thenpsutil.wait_procs(grace)and SIGKILLs any survivor. New_daemon_term_grace_seconds()reads the grace from config (floored at 0; 0 disables escalation). The recycled-PID identity guard runs first and still gates the entire path — escalation can never reach a stranger. Windows path unchanged (taskkill /Fis already a hard kill).hermes_cli/config.py:terminal.daemon_term_grace_seconds(default2.0) — config.yaml, not an env var, per the.env-secrets-only policy. (fix(tools): escalate SIGTERM→SIGKILL on browser daemon + periodic orphan reap #15008 usedBROWSER_*env vars, which is why it wasn't salvaged as-is.)TestSigkillEscalation— real SIGTERM-trapping subprocesses prove escalation kills them,grace=0spares them, well-behaved procs die on SIGTERM, the recycled-PID guard is not bypassed, and a negative grace floors to 0. Two pre-existing terminate tests pin grace to 0 (they cover SIGTERM ordering, not escalation).Validation
tests/tools/test_process_registry.pytest_browser_orphan_reaper.py+test_browser_cleanup.py(call_terminate_host_pid)grace=0Completes the process-kill-safety arc from this session (#50417 browser reaper identity, #50468 recycled-PID guard) by closing the "daemon ignores SIGTERM → leaks" gap. Credit @tkwong (Benjamin Wong) for the original escalation idea in #15008.
Infographic