fix(cron): reap task subprocess group and bound post-kill pipe drain - #21
Open
muqiao215 wants to merge 1 commit into
Open
fix(cron): reap task subprocess group and bound post-kill pipe drain#21muqiao215 wants to merge 1 commit into
muqiao215 wants to merge 1 commit into
Conversation
Provider CLIs spawned by cron/webhook one-shot tasks (e.g. `claude`) launch worker subprocesses that inherit the parent's stdout/stderr pipes. The previous cleanup only force-killed the lead PID and then called an unbounded `proc.communicate()`. When an orphaned grandchild kept a pipe write-end open, `communicate()` never saw EOF and blocked the main asyncio loop indefinitely. Symptom across the fleet: controlmesh.service stayed active (running) with Errors: 0, but every periodic task (Telegram/Lark polling, model-cache refresh, further cron jobs) froze in the morning cron window and bots stopped responding. Fix: - Start each task subprocess in its own session/process-group (`start_new_session=True` on POSIX) so the whole tree can be signalled. - Replace the lead-PID kill with a group-aware kill (`os.killpg`) plus the existing PID-tree kill as a best-effort fallback. - Bound the post-kill pipe drain (`_POST_KILL_DRAIN_SECONDS`); abandon the pipes instead of blocking the loop if descendants survive the group kill. Tests: - subprocess is started with `start_new_session=True` on POSIX - a pipe held open by a simulated orphan no longer blocks the loop - the process group (not just the lead PID) is signalled on timeout
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.
Root cause
Fleet-wide ControlMesh outage: every host's
controlmesh.serviceshowedactive (running)/Errors: 0, yet bots stopped responding to messages and the agent log froze in the morning cron window (05:09–06:40 server-local). Two of seven hosts whose morning jobs happened to succeed stayed alive.A cron/webhook one-shot job (e.g. NotebookLM cinematic video) spawns a provider CLI (
claude) which launches worker subprocesses that inherit the parent's stdout/stderr pipes. The previous timeout cleanup incron/execution.pyonly force-killed the lead PID and then called an unboundedproc.communicate(). When an orphaned grandchild kept a pipe write-end open,communicate()never observed EOF → theawaitblocked the main asyncio loop forever → Telegram/Lark polling, model-cache refresh, and all further cron jobs stopped.Evidence on
racknerd-436b0c0: main process at 85% CPU, a childclauderunning the NotebookLM task for hours, log frozen at the exact line where the job acquired thechrome_browserdependency; noCron job completedline ever followed.Fix
In
controlmesh/cron/execution.py:start_new_session=Trueon POSIX).os.killpgon the child's pgid) plus the existing PID-tree kill as a best-effort fallback — so orphaned/reparented grandchildren that hold the pipes die together._POST_KILL_DRAIN_SECONDS = 10); abandon the pipes instead of blocking the loop if anything survives the group kill.This guarantees a stuck provider subprocess can no longer freeze the event loop, regardless of
cli_timeoutor how the provider CLI spawns its workers.Tests (
tests/cron/test_execution.py)test_starts_subprocess_in_own_session_on_posix— verifiesstart_new_session=Trueis passed.test_post_kill_drain_is_bounded— a pipe held open by a simulated orphan returns in ~50ms instead of hanging (regression test for the loop deadlock).test_timeout_kills_process_group— asserts the whole process group (os.killpg) is signalled, not just the lead PID.Notes
cli/executor.py) already used a saner soft/hard kill pattern; this brings the cron/webhook one-shot path to the same safety level and additionally covers the pipe-EOF deadlock, which the foreground path avoids by not callingcommunicate()after kill.py_compile+ a standalone asyncio simulation of the bounded drain (returns in 0.06s on a permanently-blocked pipe).