Skip to content

fix(cron): reap task subprocess group and bound post-kill pipe drain - #21

Open
muqiao215 wants to merge 1 commit into
mainfrom
fix/cron-subprocess-group-kill
Open

fix(cron): reap task subprocess group and bound post-kill pipe drain#21
muqiao215 wants to merge 1 commit into
mainfrom
fix/cron-subprocess-group-kill

Conversation

@muqiao215

Copy link
Copy Markdown
Owner

Root cause

Fleet-wide ControlMesh outage: every host's controlmesh.service showed active (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 in cron/execution.py 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 observed EOF → the await blocked 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 child claude running the NotebookLM task for hours, log frozen at the exact line where the job acquired the chrome_browser dependency; no Cron job completed line ever followed.

Fix

In controlmesh/cron/execution.py:

  • Start each task subprocess in its own session/process-group (start_new_session=True on POSIX).
  • Replace the lead-PID kill with a group-aware kill (os.killpg on 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.
  • Bound the post-kill pipe drain (_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_timeout or how the provider CLI spawns its workers.

Tests (tests/cron/test_execution.py)

  • test_starts_subprocess_in_own_session_on_posix — verifies start_new_session=True is 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

  • Foreground execution (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 calling communicate() after kill.
  • Verified offline via py_compile + a standalone asyncio simulation of the bounded drain (returns in 0.06s on a permanently-blocked pipe).

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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant