Skip to content

Commit 3d07668

Browse files
authored
fix(transport): unblock read_messages when the CLI dies but orphans hold its stdout pipe
The stdout read loop treated pipe EOF as the only process-death signal. Helper processes spawned by the CLI inherit its stdout pipe, so when the CLI exits without reaping them (crash or deliberate nonzero exit) EOF never arrives and read_messages blocks forever: consumers hang with no ProcessError and no end-of-stream (#1110). On the asyncio backend process.wait() is gated on the same pipes, so the trailing exit-code check would hang identically. Add a process-exit watcher that polls returncode (visible regardless of pipe holders) and closes the stdout stream once the reader has been parked on a silent pipe for a full grace window, routing the read loop through its existing ClosedResourceError path into the exit-code check. Chunk-count and parked-on-read guards ensure a reader mid-drain or suspended at yield by consumer backpressure is never cut off, so output written before death is always delivered. Fixes #1110
1 parent 528265f commit 3d07668

3 files changed

Lines changed: 245 additions & 3 deletions

File tree

src/claude_agent_sdk/_internal/transport/subprocess_cli.py

Lines changed: 93 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,11 @@
3030
_DEFAULT_MAX_BUFFER_SIZE = 1024 * 1024 # 1MB buffer limit
3131
MINIMUM_CLAUDE_CODE_VERSION = "2.0.0"
3232

33+
# After the CLI process exits, how long the stdout pipe must stay silent with
34+
# the reader parked on it before the reader is forcibly unblocked. See
35+
# _unblock_reader_on_exit.
36+
_EXIT_STREAM_GRACE_SECONDS = 2.0
37+
3338
# Track live CLI subprocesses so we can terminate them when the parent Python
3439
# process exits. This mirrors the TypeScript SDK's parent-exit cleanup and
3540
# prevents orphaned `claude` processes from leaking when callers crash or exit
@@ -130,6 +135,12 @@ def __init__(
130135
self._stdin_stream: TextSendStream | None = None
131136
self._stderr_stream: TextReceiveStream | None = None
132137
self._stderr_task: TaskHandle | None = None
138+
self._exit_watcher_task: TaskHandle | None = None
139+
# Read-loop introspection for _unblock_reader_on_exit: whether the
140+
# reader is currently parked awaiting a stdout chunk, and how many
141+
# chunks it has consumed so far.
142+
self._awaiting_stdout = False
143+
self._stdout_chunks = 0
133144
self._ready = False
134145
self._exit_error: Exception | None = None # Track process exit errors
135146
self._max_buffer_size = (
@@ -554,6 +565,10 @@ async def connect(self) -> None:
554565
# same pattern as Query._read_task.
555566
self._stderr_task = spawn_detached(self._handle_stderr())
556567

568+
# Watch for process death so the read loop can't hang on a pipe
569+
# kept open by orphaned CLI helper processes.
570+
self._exit_watcher_task = spawn_detached(self._unblock_reader_on_exit())
571+
557572
# Setup stdin for streaming (always used now)
558573
if self._process.stdin:
559574
self._stdin_stream = TextSendStream(self._process.stdin)
@@ -621,6 +636,53 @@ def emit(line: str) -> None:
621636
# synchronous, so it is safe to run during cancellation unwind.
622637
emit(framer.flush())
623638

639+
async def _unblock_reader_on_exit(self) -> None:
640+
"""Force the stdout read loop to finish when the process is dead but
641+
stdout never reaches EOF.
642+
643+
EOF is not a reliable process-death signal: helper processes spawned
644+
by the CLI inherit its stdout pipe, and when the CLI exits without
645+
reaping them (a crash, or the deliberate nonzero exit after a fatal
646+
error) a surviving helper keeps the write end open. The read loop then
647+
blocks forever and neither the trailing ProcessError nor end-of-stream
648+
ever reaches consumers (#1110).
649+
650+
After the process exits, close the stdout stream once the reader has
651+
been parked on a silent pipe for a full grace window. Both conditions
652+
matter: a reader that is mid-drain or suspended at ``yield`` by
653+
consumer backpressure (``_awaiting_stdout`` False) is left alone, and
654+
a chunk arriving during the window (``_stdout_chunks`` moved) restarts
655+
it — so output the CLI wrote before dying is never cut off. Closing
656+
the stream sends the read loop through its existing
657+
``ClosedResourceError`` path into the normal exit-code check.
658+
"""
659+
if self._process is None:
660+
return
661+
# Poll returncode rather than awaiting wait(): on the asyncio backend
662+
# wait() itself is gated on the pipes disconnecting (the transport only
663+
# finishes once every pipe protocol reports disconnected), so in
664+
# exactly the scenario this watcher exists for, wait() blocks on the
665+
# same orphan-held pipe as the read loop. returncode is set from the
666+
# child-exit signal and becomes visible regardless of pipe holders.
667+
while self._process.returncode is None:
668+
await anyio.sleep(_EXIT_STREAM_GRACE_SECONDS)
669+
prev_chunks = self._stdout_chunks
670+
while True:
671+
await anyio.sleep(_EXIT_STREAM_GRACE_SECONDS)
672+
stream = self._stdout_stream
673+
if stream is None:
674+
return
675+
if self._stdout_chunks == prev_chunks and self._awaiting_stdout:
676+
break
677+
prev_chunks = self._stdout_chunks
678+
logger.debug(
679+
"CLI process exited but stdout never reached EOF "
680+
"(likely inherited by an orphaned child process); "
681+
"closing the stream to unblock the reader"
682+
)
683+
with suppress(Exception):
684+
await stream.aclose()
685+
624686
async def close(self) -> None:
625687
"""Close the transport and clean up resources.
626688
@@ -661,6 +723,16 @@ async def close(self) -> None:
661723
await self._stderr_task.wait()
662724
self._stderr_task = None
663725

726+
# Cancel the process-exit watcher (same pattern as stderr task)
727+
if (
728+
self._exit_watcher_task is not None
729+
and not self._exit_watcher_task.done()
730+
):
731+
self._exit_watcher_task.cancel()
732+
with suppress(Exception):
733+
await self._exit_watcher_task.wait()
734+
self._exit_watcher_task = None
735+
664736
# Close stdin stream (hold the write lock to prevent a race with
665737
# concurrent writes). Bounded: a writer blocked on a full stdin
666738
# pipe must not pin the shielded scope forever.
@@ -780,7 +852,19 @@ def guard(length: int) -> None:
780852
)
781853

782854
try:
783-
async for chunk in self._stdout_stream:
855+
stream_iter = aiter(self._stdout_stream)
856+
while True:
857+
# Flag the park-on-read state (and count chunks) so
858+
# _unblock_reader_on_exit can tell an idle pipe apart from a
859+
# reader suspended at `yield` by consumer backpressure.
860+
self._awaiting_stdout = True
861+
try:
862+
chunk = await anext(stream_iter)
863+
except StopAsyncIteration:
864+
break
865+
finally:
866+
self._awaiting_stdout = False
867+
self._stdout_chunks += 1
784868
for line in framer.push(chunk):
785869
guard(len(line))
786870
data = _parse_stdout_line(line)
@@ -808,9 +892,15 @@ def guard(length: int) -> None:
808892
if data is not None:
809893
yield data
810894

811-
# Check process completion and handle errors
895+
# Check process completion and handle errors. Prefer the already-set
896+
# returncode: when _unblock_reader_on_exit closed the stream, wait()
897+
# would block right here on the asyncio backend (it is gated on the
898+
# same orphan-held pipe the watcher just detected — see that method).
812899
try:
813-
returncode = await self._process.wait()
900+
if self._process.returncode is not None:
901+
returncode = self._process.returncode
902+
else:
903+
returncode = await self._process.wait()
814904
except Exception:
815905
returncode = -1
816906

tests/test_subprocess_buffering.py

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -342,6 +342,7 @@ async def _test() -> None:
342342
transport = SubprocessCLITransport(prompt="test", options=make_options())
343343
transport._stdout_stream = stream
344344
transport._process = MagicMock()
345+
transport._process.returncode = None
345346
transport._process.wait = AsyncMock(return_value=0)
346347

347348
messages: list[dict[str, Any]] = []
@@ -372,6 +373,7 @@ async def _test() -> None:
372373
transport = SubprocessCLITransport(prompt="test", options=make_options())
373374
transport._stdout_stream = stream
374375
transport._process = MagicMock()
376+
transport._process.returncode = None
375377
transport._process.wait = AsyncMock(return_value=0)
376378

377379
messages: list[dict[str, Any]] = []
@@ -391,6 +393,7 @@ def _collect(self, chunks: list[str], **opts: object) -> list[Any]:
391393
async def _run() -> None:
392394
transport = SubprocessCLITransport(prompt="t", options=make_options(**opts))
393395
transport._process = MagicMock()
396+
transport._process.returncode = None
394397
transport._process.wait = AsyncMock(return_value=0)
395398
transport._stdout_stream = MockTextReceiveStream(chunks)
396399
transport._stderr_stream = MockTextReceiveStream([])
@@ -523,6 +526,7 @@ async def __anext__(self) -> str:
523526

524527
transport = SubprocessCLITransport(prompt="t", options=make_options())
525528
transport._process = MagicMock()
529+
transport._process.returncode = None
526530
transport._process.wait = AsyncMock(return_value=0)
527531
transport._stdout_stream = ClosingStream(
528532
[json.dumps({"type": "result", "subtype": "success"})]

tests/test_transport.py

Lines changed: 148 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2219,3 +2219,151 @@ async def _test() -> None:
22192219
await proc.wait()
22202220

22212221
anyio.run(_test)
2222+
2223+
2224+
class TestReaderUnblockOnProcessExit:
2225+
"""Regression tests for #1110: process death must end read_messages even
2226+
when stdout never reaches EOF.
2227+
2228+
Helper processes spawned by the CLI inherit its stdout pipe; if the CLI
2229+
dies without reaping them, EOF never arrives. On the asyncio backend
2230+
``process.wait()`` is gated on the same pipes, so the mocks here enforce
2231+
the real contract: ``wait()`` never returns, only ``returncode`` is set.
2232+
"""
2233+
2234+
class _PipeHeldOpenStream:
2235+
"""Stdout stream whose EOF never arrives (orphan holds the pipe).
2236+
2237+
Yields the given chunks, then parks forever — until aclose(), after
2238+
which the parked (or next) read raises ClosedResourceError, exactly
2239+
like a real TextReceiveStream.
2240+
"""
2241+
2242+
def __init__(self, chunks: list[str]) -> None:
2243+
self._chunks = list(chunks)
2244+
self._closed = anyio.Event()
2245+
2246+
def __aiter__(self) -> "TestReaderUnblockOnProcessExit._PipeHeldOpenStream":
2247+
return self
2248+
2249+
async def __anext__(self) -> str:
2250+
if self._closed.is_set():
2251+
raise anyio.ClosedResourceError
2252+
if self._chunks:
2253+
return self._chunks.pop(0)
2254+
await self._closed.wait()
2255+
raise anyio.ClosedResourceError
2256+
2257+
async def aclose(self) -> None:
2258+
self._closed.set()
2259+
2260+
class _ExitedProcessWithHeldPipes:
2261+
"""Process that has exited, but whose wait() never completes.
2262+
2263+
Mirrors asyncio's behavior when an inherited pipe outlives the child:
2264+
returncode is set from the exit signal while wait() stays blocked on
2265+
pipe disconnection. The fix must not rely on wait().
2266+
"""
2267+
2268+
def __init__(self, returncode: int) -> None:
2269+
self.returncode = returncode
2270+
2271+
async def wait(self) -> int:
2272+
await anyio.sleep(3600)
2273+
return self.returncode
2274+
2275+
def _make_transport(self, returncode: int, chunks: list[str]):
2276+
transport = SubprocessCLITransport(prompt="test", options=make_options())
2277+
transport._process = self._ExitedProcessWithHeldPipes(returncode) # type: ignore[assignment]
2278+
transport._stdout_stream = self._PipeHeldOpenStream(chunks) # type: ignore[assignment]
2279+
return transport
2280+
2281+
def test_crash_with_orphan_held_pipe_raises_process_error(self, monkeypatch):
2282+
"""CLI died with exit 1, orphan holds stdout: must raise, not hang."""
2283+
from claude_agent_sdk._errors import ProcessError
2284+
from claude_agent_sdk._internal._task_compat import spawn_detached
2285+
from claude_agent_sdk._internal.transport import subprocess_cli
2286+
2287+
monkeypatch.setattr(subprocess_cli, "_EXIT_STREAM_GRACE_SECONDS", 0.05)
2288+
2289+
async def _test():
2290+
transport = self._make_transport(
2291+
returncode=1,
2292+
chunks=['{"type": "assistant", "message": {"content": []}}\n'],
2293+
)
2294+
watcher = spawn_detached(transport._unblock_reader_on_exit())
2295+
received = []
2296+
try:
2297+
with anyio.fail_after(5):
2298+
with pytest.raises(ProcessError, match="exit code 1"):
2299+
async for message in transport.read_messages():
2300+
received.append(message)
2301+
finally:
2302+
if not watcher.done():
2303+
watcher.cancel()
2304+
# Output produced before death must still be delivered.
2305+
assert len(received) == 1
2306+
assert received[0]["type"] == "assistant"
2307+
2308+
anyio.run(_test)
2309+
2310+
def test_clean_exit_with_orphan_held_pipe_ends_stream(self, monkeypatch):
2311+
"""CLI exited 0 but orphan holds stdout: stream must end cleanly."""
2312+
from claude_agent_sdk._internal._task_compat import spawn_detached
2313+
from claude_agent_sdk._internal.transport import subprocess_cli
2314+
2315+
monkeypatch.setattr(subprocess_cli, "_EXIT_STREAM_GRACE_SECONDS", 0.05)
2316+
2317+
async def _test():
2318+
transport = self._make_transport(
2319+
returncode=0,
2320+
chunks=['{"type": "result", "subtype": "success"}\n'],
2321+
)
2322+
watcher = spawn_detached(transport._unblock_reader_on_exit())
2323+
received = []
2324+
try:
2325+
with anyio.fail_after(5):
2326+
async for message in transport.read_messages():
2327+
received.append(message)
2328+
finally:
2329+
if not watcher.done():
2330+
watcher.cancel()
2331+
assert len(received) == 1
2332+
assert received[0]["type"] == "result"
2333+
2334+
anyio.run(_test)
2335+
2336+
def test_backpressure_is_not_mistaken_for_idle_pipe(self, monkeypatch):
2337+
"""A reader parked at yield (slow consumer) must not be force-closed.
2338+
2339+
The consumer takes longer than the grace window to process each
2340+
message while more chunks are still arriving; every chunk must be
2341+
delivered before the trailing ProcessError.
2342+
"""
2343+
from claude_agent_sdk._errors import ProcessError
2344+
from claude_agent_sdk._internal._task_compat import spawn_detached
2345+
from claude_agent_sdk._internal.transport import subprocess_cli
2346+
2347+
monkeypatch.setattr(subprocess_cli, "_EXIT_STREAM_GRACE_SECONDS", 0.05)
2348+
2349+
async def _test():
2350+
chunks = [
2351+
f'{{"type": "assistant", "message": {{"content": [], "i": {i}}}}}\n'
2352+
for i in range(5)
2353+
]
2354+
transport = self._make_transport(returncode=1, chunks=chunks)
2355+
watcher = spawn_detached(transport._unblock_reader_on_exit())
2356+
received = []
2357+
try:
2358+
with anyio.fail_after(10):
2359+
with pytest.raises(ProcessError, match="exit code 1"):
2360+
async for message in transport.read_messages():
2361+
received.append(message)
2362+
# Slower than the grace window.
2363+
await anyio.sleep(0.15)
2364+
finally:
2365+
if not watcher.done():
2366+
watcher.cancel()
2367+
assert len(received) == 5
2368+
2369+
anyio.run(_test)

0 commit comments

Comments
 (0)