bug: claude subprocess stdout inherited by MCP child processes — leaks reader threads, eventually wedges agent-server

[vybe]

Summary

When agent-server spawns a claude --print headless task, the subprocess's stdout pipe is inherited by every child process claude spawns — most consequentially the long-lived MCP server processes (npm exec moltbook-mcp, npm exec @peng-shawn/mermaid-mcp-server, npm exec aistudio-mcp-server, etc.). When claude exits, those children keep the pipe's write end open, so the agent-server's stdout reader thread blocks on a read() waiting for an EOF that never arrives. The pgroup-kill / pipe-close unwind code logs 1 reader thread(s) leaked … continuing anyway, but the leaked thread never exits — it accumulates. After enough failed/empty-result/long-running tasks, the agent-server's HTTP-handling thread pool starves and stops responding to all requests (including /health), wedging the entire agent until restarted.

This is the underlying root cause of the failures classified by #516/#517 (signal-kill 504) and #520/#521 (clean-exit empty-result 502). Those PRs made the symptoms honest — return the right HTTP status, log the right message — but neither prevents the leak. Every cron tick that hits the failure path adds another stuck thread, and eventually the operator sees the UI hang for ~5 minutes (per-agent calls timing out) and has to restart the affected agent container.

Component

Agent Runtime (agent_server.services.claude_code subprocess spawn; agent_server.utils.subprocess_pgroup reader threads)

Priority

P2 — workaround exists (restart agent container; documented in #517 and #521 follow-ups), but the leak is silent and recurring on every agent that uses MCP servers and runs heavy enough headless tasks. UI symptoms only appear after enough threads accumulate, which makes the connection between the leak and the wedge non-obvious for operators.

Error

In agent-server logs, repeated for every failed/empty-result task:

[Subprocess] Reader thread(s) stuck after process exit (pid=NNNNN, stuck_count=1)
            — killing process group and closing pipes to unwind
[Subprocess] 1 reader thread(s) leaked for pid=NNNNN after close+killpg; continuing anyway
[Headless Task] Error reading stdout: I/O operation on closed file.

The thread is "closed" from the agent-server's POV (file handle replaced) but the OS-level read syscall is still blocked because the pipe's write end is still open in a different process (an MCP child). When the leak count crosses some threshold (observed: dozens of accumulated stuck threads over days), the agent-server's HTTP thread pool exhausts:

• /health requests stop being served (5+ second timeout, returns no response)
• Backend's services.agent_client opens its circuit breaker on the agent
• Trinity UI requests that fan out to per-agent endpoints (timeline, dashboards, agent detail) hang waiting on the now-unreachable agent
• Cumulative effect: UI loads take 5+ minutes per page, agent appears "down"
• The agent-server process itself is alive and S (sleeping) on a futex — not crashed, just out of free threads

Location

• File: docker/base-image/agent_server/services/claude_code.py — subprocess spawn site
• File: docker/base-image/agent_server/utils/subprocess_pgroup.py — pgroup-kill + reader thread cleanup (where "1 reader thread(s) leaked … continuing anyway" originates)

Root Cause

POSIX file descriptors are inherited across fork() + execve() by default. When agent-server spawns claude via asyncio.create_subprocess_exec(stdout=PIPE, ...), Python wires the child end of the pipe into claude's stdout. claude then spawns its own subprocesses (Node MCP servers, sometimes shells) without explicitly closing inherited fds — meaning the same pipe write end is duplicated into every grandchild.

When claude exits cleanly (or is signal-killed), the kernel closes claude's copy of the pipe. But it does not close the copies held by the MCP grandchildren. The pipe's reference count remains > 0, so the read end never sees EOF. The agent-server's reader thread sits in a blocking read() on that fd indefinitely.

subprocess_pgroup's "killpg + close pipes" recovery path closes the file handle in the agent-server process, but the underlying kernel fd is still held by the read syscall, which won't return until either:

1. The MCP child also closes/exits (could be never — MCP servers are designed to be long-lived), or
2. The thread is forcibly killed (which Python can't do safely), or
3. The agent-server process itself dies

Hence the "leaked" log message and the slow accumulation.

Reproduction Steps

1. Configure an agent with at least one MCP server that spawns a long-lived process (the standard Trinity agents with the moltbook / mermaid / aistudio MCP servers all qualify).
2. Schedule a heavy headless task (anything with claude --print --output-format stream-json --verbose against a non-trivial skill).
3. Cause the task to fail through either path that fix(agent): classify signal-killed claude exits as 504, not fake auth failure (#516) #517 / fix(agent): classify clean-exit empty-result as 502, not silent success (#520) #521 now classify (timeout SIGKILL, or clean-exit-with-empty-result). The simplest synthetic trigger: set the schedule timeout shorter than the skill's runtime so SIGKILL fires.
4. Observe in agent-server logs: Reader thread(s) stuck after process exit … 1 reader thread(s) leaked.
5. Repeat the failing schedule a few dozen times (or just leave a broken cron in place for a day or two).
6. Observe: agent-server eventually stops responding to /health (timeout, no response). Backend's circuit breaker opens. Trinity UI views that depend on per-agent calls take 5+ minutes to render.
7. Confirm the agent process is alive but blocked: cat /proc/<pid>/wchan shows futex_wait_queue, status S, threads = (small number, but the thread pool internal to the HTTP server is exhausted).
8. Restart the agent container — everything recovers immediately. Repeat from step 3.

Suggested Fix

Two complementary changes; the first is the actual fix, the second is defense in depth.

Primary: prevent fd inheritance on the stdout pipe

Set FD_CLOEXEC on the agent-server's end of the stdout pipe before spawning claude (so Python's pipe still works in claude itself, but anything claude spawns via fork+exec won't inherit the duplicate write end). Pseudo-fix:

# In claude_code.py, around the asyncio.create_subprocess_exec call:
import fcntl

# Create the pipe ourselves so we control the flags.
read_fd, write_fd = os.pipe()
# Set close-on-exec on the WRITE end — this is the one claude's grandchildren
# would otherwise inherit. claude itself uses it via stdin/stdout dup,
# which dup2 strips CLOEXEC — so claude's own stdout still works,
# but its forked-then-exec'd children won't carry the fd into their address space.
flags = fcntl.fcntl(write_fd, fcntl.F_GETFD)
fcntl.fcntl(write_fd, fcntl.F_SETFD, flags | fcntl.FD_CLOEXEC)

proc = await asyncio.create_subprocess_exec(
    *cmd,
    stdin=asyncio.subprocess.DEVNULL,
    stdout=write_fd,        # claude writes here (CLOEXEC stripped by dup2)
    stderr=asyncio.subprocess.PIPE,
    ...,
)
os.close(write_fd)  # parent doesn't need it; only the read_fd matters now
# Wrap read_fd as the asyncio reader as usual

Note: standard asyncio.create_subprocess_exec(stdout=PIPE) does not set CLOEXEC on the parent's pipe ends in a way that protects against this, because dup2 clears CLOEXEC during the child's stdout setup. The fix has to happen on the explicit pipe before the spawn.

Defense in depth: thread-pool sizing + observability

• Bound the number of leaked reader threads agent-server tolerates before raising a startup-style health-check failure (so the wedge surfaces as a hard fail instead of silent UI hangs).
• Expose a metric (agent_server_leaked_reader_threads) so operators can alert on accumulation before exhaustion.
• Add a unit test: spawn a fake claude that itself spawns a child holding stdout open, then have the parent exit; assert that the agent-server reaper closes cleanly within N seconds and does not leak the reader thread.

Environment

• Trinity version: observed on dev 0b3bb58 (post fix(agent): classify signal-killed claude exits as 504, not fake auth failure (#516) #517 + fix(agent): classify clean-exit empty-result as 502, not silent success (#520) #521); the underlying bug pre-exists both fixes — they classify the symptom honestly but neither prevents the leak.
• Affected: every agent runtime that (a) uses MCP servers as long-lived child processes and (b) hits the failed-headless-task code path frequently. In practice: most production agents with non-trivial skills.

Related

• bug: SIGKILL/timeout terminations of claude subprocess misclassified as authentication failure #516 / fix(agent): classify signal-killed claude exits as 504, not fake auth failure (#516) #517 — signal-kill misclassification fix. Symptom of this bug.
• bug: agent_server reports task as 'completed successfully' when claude exits 0 with no result message #520 / fix(agent): classify clean-exit empty-result as 502, not silent success (#520) #521 — clean-exit empty-result misclassification fix. Symptom of this bug.
• bug: agent-server.py spins at ~83% CPU after claude CLI subprocess becomes defunct #407 — subprocess zombie reaper. Adjacent plumbing; the reaper handles process-tree cleanup but not fd inheritance.
• bug: long-running backend→agent task calls dropped when uvicorn worker recycles #408 — long-running backend→agent task calls dropped when uvicorn worker recycles. Likely amplifies the visibility of this bug (lost responses → backend keeps polling → more concurrent open requests → faster thread-pool exhaustion).
• File: docker/base-image/agent_server/services/claude_code.py
• File: docker/base-image/agent_server/utils/subprocess_pgroup.py

[vybe]

Diagnosis pivot — root cause is not pipe inheritance

After reading the existing post-#407 cleanup code (subprocess_pgroup.drain_reader_threads) and looking at the actual production behavior more carefully, the original framing in this issue and the companion evidence file is directionally close but mislocates the bug. Documenting here so the fix matches reality.

What's actually wrong

The dominant user-visible symptom — agent-server's /health timing out, backend circuit breaker opening, UI fan-out hanging for 5+ minutes — is caused by synchronous process.wait() calls blocking the asyncio event loop in async endpoints, not by pipe FD inheritance leaking threads.

Three concrete hazards in agent_server:

Site	What's blocking
docker/base-image/agent_server/routers/chat.py:260 (async def terminate_execution)	calls registry.terminate() directly; that does up to 5s SIGINT grace + 2s SIGKILL grace = 7s of process.wait() on the event loop
docker/base-image/agent_server/services/claude_code.py:689-690 (execute_claude_code outer asyncio.TimeoutError handler)	_terminate_process_group(...) + _safe_close_pipes(...) synchronously, up to 4s process.wait() on the event loop
docker/base-image/agent_server/services/claude_code.py:1296-1297 (execute_headless_task outer asyncio.TimeoutError handler)	same pattern

While any of those is in flight, the event loop cannot serve any other request, including /health. Backend's 5s /health timeout fires, the circuit breaker opens, and every UI page that fans out to per-agent calls hangs.

The trinity-ops-agent investigation also confirmed steady-state thread count is 4 (not climbing) and that agent-server's pipe FDs are clean — i.e. the existing start_new_session=True + killpg + safe_close_pipes path actually does work in the vast majority of cases. The 1 reader thread(s) leaked log fires only ~4 times over 4.5 hours per affected agent, and the in-flight reader does eventually unwind. So the leak is real but rare and not the wedge mechanism.

Why the proposed FD_CLOEXEC fix wouldn't have helped

Two reasons:

1. It doesn't actually prevent inheritance. The fix sets FD_CLOEXEC on the parent's write_fd before subprocess.Popen(stdout=write_fd). But Popen does dup2(write_fd, 1) in the forked child, and dup2 clears FD_CLOEXEC on the target fd by POSIX spec. After exec, claude has fd 1 → pipe with no FD_CLOEXEC. When claude fork()s npm exec / node MCP grandchildren, fd 1 is inherited unchanged. The CLOEXEC bit on the parent's write_fd never propagates down the chain. (The same paragraph in this issue's "Suggested Fix" section actually admits "dup2 strips CLOEXEC during the child's stdout setup" — that's exactly why setting CLOEXEC on write_fd doesn't help.)

2. Even if it did work, it wouldn't fix the wedge symptom. The wedge is the event-loop block from sync process.wait() in async paths. Preventing pipe inheritance would only address the rare leak-log path, not the much more frequent terminate-path block.

The fix

Wrap the three sync-blocking calls in loop.run_in_executor(...) so they don't block the event loop:

# routers/chat.py: terminate_execution
loop = asyncio.get_event_loop()
result = await loop.run_in_executor(None, registry.terminate, execution_id)

# claude_code.py: outer asyncio.TimeoutError handler (both call sites)
await loop.run_in_executor(
    None,
    functools.partial(_terminate_process_group, process, graceful_timeout=2, pgid=process_pgid)
)
await loop.run_in_executor(None, _safe_close_pipes, process)

Tiny diff. The pipe inheritance fragility remains as a slow-burn cleanup item but no longer translates into a user-visible wedge.

Follow-up (separate issue)

Pipe inheritance / leaked-reader-thread cleanup as defense-in-depth (non-blocking reader I/O via select, prctl(PR_SET_CHILD_SUBREAPER) + tree-walk kill, leaked-thread metric) is still worth doing — just not as part of #523. Will file as a P3 follow-up after this lands.

Working on it now on feature/523-async-terminate-cleanup.

[vybe]

/claim

[github-actions]

Claimed by @vybe. Label status-in-progress added.

Next steps:

1. Create branch: feature/523-<short-slug>
2. Implement the feature
3. Open a PR with Fixes #523

[vybe]

Fix landed directly on `dev` as commit `c43748d`:

fix(agent): off-load synchronous terminate cleanup off the event loop (#523)

Three call sites wrapped in `loop.run_in_executor`:

• `docker/base-image/agent_server/routers/chat.py` — `terminate_execution` → `registry.terminate`
• `docker/base-image/agent_server/services/claude_code.py` — outer `asyncio.TimeoutError` handler in `execute_claude_code` (chat path)
• `docker/base-image/agent_server/services/claude_code.py` — outer `asyncio.TimeoutError` handler in `execute_headless_task` (task path)

Plus regression test `tests/unit/test_terminate_async_executor.py` asserting `registry.terminate` runs on a non-event-loop thread and event-loop yield stays sub-50ms while terminate is in flight.

Validation pending on cornelius-m once the new base image ships:

• /health latency under load (should stay <100ms while a terminate is in flight)
• circuit breaker should stop opening on heartbeat schedule timeouts
• UI fan-out pages should load in normal time (sub-second) instead of 5+ minutes

Pipe-inheritance fragility (the original framing of this issue) remains as a defense-in-depth item — non-blocking reader I/O, leaked-thread metric — and should be filed as a P3 follow-up.

[vybe]

Update after deeper investigation (linking #531)

A round of live diagnosis on an instance running long agentic tasks reframed parts of this issue. Two of this issue's original claims need correcting, and the actual user-visible bug it was supposed to fix is now covered by a separate, simpler fix:

What we found

1. Threads are not actually accumulating. This issue's summary stated that leaked reader threads pile up until the agent-server's HTTP thread pool starves. On an instance with 12 h of uptime and 12 consecutive empty-result failures (35 "reader thread(s) leaked … continuing anyway" log lines), /proc/<agent-server-pid>/task/ showed only 2 OS threads alive. The Python Thread objects do exit shortly after safe_close_pipes(process) runs — readline() raises ValueError, the thread unwinds, the OS reaps it. The "leaked" log message is a misnomer; it's a "still alive at the moment we checked" state, not a permanent leak.

2. The cumulative-leak → wedge theory is therefore not what's happening. The 12-min wedge that prompted this issue is more plausibly explained by either (a) a transient HTTP thread-pool pressure during a single in-flight task, or (b) a now-fixed bug in the recovery path. We have not been able to reproduce the wedge since the most recent deploy despite 12 h of identical failure conditions. Worth keeping an eye on, but it is not the load-bearing claim.

3. The actual cause of the empty-result symptom is a drain-then-close ordering bug in drain_reader_threads — the recovery path closes the kernel pipe before the reader can drain whatever's already in the buffer, so the final {"type":"result"} line goes into the bit bucket. Filed as bug: drain_reader_threads closes stdout pipe before reader can drain backlog — silently loses final result line on long agentic tasks #531 with a small (~6-line) reorder fix that lets the reader drain naturally after grandchildren are killed, before any force-close. That fix is independent of FD_CLOEXEC and resolves the user-visible empty-result regardless of whether grandchildren ever inherited the stdout FD.

What this issue is still useful for

The FD_CLOEXEC work sketched here is still correct subprocess hygiene, just no longer urgent:

• Performance: with FD_CLOEXEC on the stdout pipe write end, EOF arrives the moment claude exits → reader drains in <1 s → bug: drain_reader_threads closes stdout pipe before reader can drain backlog — silently loses final result line on long agentic tasks #531's recovery path never fires. Without it, the recovery path triggers on every long task and adds ~5 s to completion.
• Hygiene: long-lived MCP grandchildren genuinely shouldn't be inheriting ancestor pipe FDs. The dup-strips-CLOEXEC subtlety is a real footgun for anyone reading this code later.
• Defense in depth: if bug: drain_reader_threads closes stdout pipe before reader can drain backlog — silently loses final result line on long agentic tasks #531's drain logic ever regresses, FD_CLOEXEC quietly removes one of the conditions that makes the recovery path necessary.

Suggested re-scoping

• Demote priority P2 → P3.
• Reframe the body around perf + hygiene rather than "wedges the agent-server."
• Block on bug: drain_reader_threads closes stdout pipe before reader can drain backlog — silently loses final result line on long agentic tasks #531 (the actual bug fix); land FD_CLOEXEC after bug: drain_reader_threads closes stdout pipe before reader can drain backlog — silently loses final result line on long agentic tasks #531 has been observed working in production for a release or two.

Happy to do the body rewrite if a maintainer wants — leaving the labels and title alone for now in case there's reason to keep the original framing visible in history.