Skip to content

knowledge LLM pool orphans its worker's helpers and then hangs on the reap (pid-scoped kill + bare wait) #6124

Description

@SebastianYuSun

Summary

knowledge/llm_pool.py's CCWorker spawns claude -p with PIPE stdio and no
start_new_session, then cleans up with proc.kill() followed by an unbounded
await proc.wait() — in both shutdown() (llm_pool.py:523) and
reset_conversation() (llm_pool.py:543).

Two failures follow, and the first causes the second:

  1. Leak. proc.kill() signals one pid. The helpers claude -p forks survive and are
    re-parented to init.
  2. Hang. Those survivors inherited the stdout/stderr pipe write ends, and asyncio's
    Process.wait() does not complete until every pipe disconnects — so the unbounded
    wait() never returns.

This is the class fixed for other sites in #5989#6003#6005. #6005's own body names the
mechanism — "or a surviving descendant holding the pipes — can hang the calling task
indefinitely"
— but its site enumeration lists only source_providers.py and
resolve_once.py. These two sites were not enumerated, and they have an extra defect the
others did not: the tree is never killed at all.

Measured, not inferred

Driving the real CCWorker through each cleanup path, with a stand-in claude on PATH that
forks one child and then lingers (survival read from /proc/<pid>/stat, never from a
kill() return value):

arm cleanup returned? elapsed survivors at ppid 1
shutdown() no 20.1 s (hit a 20 s bound) 1
reset_conversation() no 20.1 s 1
proposed fix yes 0.0 s 0

In the shutdown() arm the helper moved from ppid 19081 (the worker) to ppid 1 across
the call, while the call itself never returned.

The root cause is on the spawn side, and this is the part that matters for the fix:
child_pgid == own_pgid == 19046 in both defect arms. The worker shares the gateway's own
process group — which is exactly the case platform_compat.kill_and_reap deliberately
skips its group kill for:

A child sharing the caller's own process group (spawned without start_new_session) has no
tree of its own to signal — the group kill is skipped for it and the pid-scoped kill()
below covers it

So converting the call to kill_and_reap alone would not fix this. The
start_new_session flag at the spawn is load-bearing. In the fix arm child_pgid (21016) is
distinct from own_pgid (19046) and the reap completes immediately with zero survivors.

This is a deviation from an established convention, not a new opinion

Three places in this repo already state the rule, one of them for this exact binary:

  • apps/builtins/auto_improvement/spine/agent_runner.py:653"claude -p forks an editor +
    helpers; a plain popen.kill() only signals the parent and leaves the children running
    (the symptom we hit: agents kept costing money after Stop)."
    Fixed there with
    kill_process_tree.
  • apps/registry.py:2793"Killing only the immediate child with proc.kill() re-parents
    those grandchildren, so repeated timeouts leak processes. … Callers MUST spawn the child
    with start_new_session."
  • platform_compat.py:3117 — the same_group carve-out quoted above.

Why it matters

shutdown() runs on pool teardown; reset_conversation() runs on every
calls_since_reset rollover, i.e. routinely during a large ingestion. Each one currently
leaves a live claude -p --permission-mode bypassPermissions subtree with no gateway-side
handle on it — the "kept costing money after Stop" symptom agent_runner recorded — and,
because of the pipe-holding survivor, can wedge the calling task rather than returning.

Proposed fix

Two lines of mechanism, both already in-tree:

  1. start_new_session=platform_compat.IS_POSIX at the spawn, so the worker leads its own
    group and its descendants are reachable as a tree.
  2. Route both cleanup paths through platform_compat.kill_and_reap — tree kill, pid
    fallback, bounded pipe-draining communicate() reap, cancellation-shielded — the same
    helper fix: bound PIPE-stdio child reaps via shared kill_and_reap (#5989) #6003 introduced and Route the two residual kill-then-bare-wait reap sites through kill_and_reap #6005 converted its sites to.

Regression pins, following #6005's pattern (a probe whose wait() is counted, plus a patched
kill_process_tree_async so no test reaches a real killpg): the spawn must request a new
session, and each cleanup must reach the tree-kill helper and drain via communicate()
without touching wait(). All three fail on unmodified code.

Related sites (inventory for triage, not part of the proposed fix)

A static audit of 216 spawn/cleanup sites found this same class — child can fork descendants,
cleanup signals only the immediate pid — at these places. Listed so the class can be triaged
as a whole the way #5989 did; I am proposing to fix only llm_pool.py here.

site spawned note
dashboard/handlers/core.py:1079 bash -c (xcode-select / brew / pipx) POST /api/stt/install; reports "Install timed out" and re-arms while the installer keeps mutating the host
dep_sync.py:819, :951 pip install into the live gateway's site-packages slack/gateway.py:2190 already documents the hazard for this argv
cli_server.py:1447 sh -c <wheel update> installer continues after "Installer timed out" + exit(1)
frontend.py:701 npm ci / npm install frontend.py:335 in the same file already uses kill_process_tree
dashboard/handlers/files.py:4001, :4006 git status / git diff dashboard polls this, so it recurs per poll
apps/routes.py:2515 git clone the sibling clone chokepoint in apps/registry.py does route through _communicate_with_timeout
dashboard/handlers/sessions.py:662, :922, agents.py:1087 kiro-cli agents.py:1087 already spawns with a new session and simply never group-signals it
dashboard/handlers/updates.py:593, :1322, :1394 git fetch / git pull :1322 is the fail-closed pre-apply fetch
cloud/aws.py:260, cloud/wizard.py:103, :107 aws … Ctrl+C reports an interrupted deploy that is still deploying
md_notebook/git_ops.py:411, ops_mission_control/backend/ledger_sync.py:378 git push/fetch killed with no reap at all
cli_setup.py:202, :213, env.py:568, cli.py:277 npm install, npx electron-builder, bash <script> install continues after the caller returns

There is also a structural amplifier worth a maintainer decision separately: on Linux
wrap_argv returns [sys.executable, <launcher>.py, *argv] and that launcher forks — the
parent writes the uid/gid maps and blocks in waitpid with no signal handler installed
(sandbox.py:2057, sandbox.py:1584). So for every sandbox-wrapped spawn the real tool is
already a grandchild, independent of whether it forks, and a pid-scoped kill there reaps
the map-writer. I could not exercise that layer: the host I measured on returns EPERM for
unshare(CLONE_NEWUSER), so the numbers above come from the unsandboxed path and are a
conservative lower bound — the default configuration adds one more layer between
proc.pid and the tool.

Metadata

Metadata

Assignees

No one assigned

    Labels

    area: coreCore runtime utilities, config/paths, process singletonsbugSomething is not working

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions