Skip to content

Commit 86def6f

Browse files
committed
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 f8b9ec9 commit 86def6f

3 files changed

Lines changed: 244 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
@@ -38,6 +38,11 @@
3838
# verbatim. See _reject_windows_batch_cli / _reject_windows_cmd_metacharacters.
3939
_CMD_EXE_METACHARACTERS = '&|<>^%!"'
4040

41+
# After the CLI process exits, how long the stdout pipe must stay silent with
42+
# the reader parked on it before the reader is forcibly unblocked. See
43+
# _unblock_reader_on_exit.
44+
_EXIT_STREAM_GRACE_SECONDS = 2.0
45+
4146
# Track live CLI subprocesses so we can terminate them when the parent Python
4247
# process exits. This mirrors the TypeScript SDK's parent-exit cleanup and
4348
# prevents orphaned `claude` processes from leaking when callers crash or exit
@@ -138,6 +143,12 @@ def __init__(
138143
self._stdin_stream: TextSendStream | None = None
139144
self._stderr_stream: TextReceiveStream | None = None
140145
self._stderr_task: TaskHandle | None = None
146+
self._exit_watcher_task: TaskHandle | None = None
147+
# Read-loop introspection for _unblock_reader_on_exit: whether the
148+
# reader is currently parked awaiting a stdout chunk, and how many
149+
# chunks it has consumed so far.
150+
self._awaiting_stdout = False
151+
self._stdout_chunks = 0
141152
self._ready = False
142153
self._exit_error: Exception | None = None # Track process exit errors
143154
self._max_buffer_size = (
@@ -752,6 +763,10 @@ async def connect(self) -> None:
752763
# same pattern as Query._read_task.
753764
self._stderr_task = spawn_detached(self._handle_stderr())
754765

766+
# Watch for process death so the read loop can't hang on a pipe
767+
# kept open by orphaned CLI helper processes.
768+
self._exit_watcher_task = spawn_detached(self._unblock_reader_on_exit())
769+
755770
# Setup stdin for streaming (always used now)
756771
if self._process.stdin:
757772
self._stdin_stream = TextSendStream(self._process.stdin)
@@ -819,6 +834,53 @@ def emit(line: str) -> None:
819834
# synchronous, so it is safe to run during cancellation unwind.
820835
emit(framer.flush())
821836

837+
async def _unblock_reader_on_exit(self) -> None:
838+
"""Force the stdout read loop to finish when the process is dead but
839+
stdout never reaches EOF.
840+
841+
EOF is not a reliable process-death signal: helper processes spawned
842+
by the CLI inherit its stdout pipe, and when the CLI exits without
843+
reaping them (a crash, or the deliberate nonzero exit after a fatal
844+
error) a surviving helper keeps the write end open. The read loop then
845+
blocks forever and neither the trailing ProcessError nor end-of-stream
846+
ever reaches consumers (#1110).
847+
848+
After the process exits, close the stdout stream once the reader has
849+
been parked on a silent pipe for a full grace window. Both conditions
850+
matter: a reader that is mid-drain or suspended at ``yield`` by
851+
consumer backpressure (``_awaiting_stdout`` False) is left alone, and
852+
a chunk arriving during the window (``_stdout_chunks`` moved) restarts
853+
it — so output the CLI wrote before dying is never cut off. Closing
854+
the stream sends the read loop through its existing
855+
``ClosedResourceError`` path into the normal exit-code check.
856+
"""
857+
if self._process is None:
858+
return
859+
# Poll returncode rather than awaiting wait(): on the asyncio backend
860+
# wait() itself is gated on the pipes disconnecting (the transport only
861+
# finishes once every pipe protocol reports disconnected), so in
862+
# exactly the scenario this watcher exists for, wait() blocks on the
863+
# same orphan-held pipe as the read loop. returncode is set from the
864+
# child-exit signal and becomes visible regardless of pipe holders.
865+
while self._process.returncode is None:
866+
await anyio.sleep(_EXIT_STREAM_GRACE_SECONDS)
867+
prev_chunks = self._stdout_chunks
868+
while True:
869+
await anyio.sleep(_EXIT_STREAM_GRACE_SECONDS)
870+
stream = self._stdout_stream
871+
if stream is None:
872+
return
873+
if self._stdout_chunks == prev_chunks and self._awaiting_stdout:
874+
break
875+
prev_chunks = self._stdout_chunks
876+
logger.debug(
877+
"CLI process exited but stdout never reached EOF "
878+
"(likely inherited by an orphaned child process); "
879+
"closing the stream to unblock the reader"
880+
)
881+
with suppress(Exception):
882+
await stream.aclose()
883+
822884
async def close(self) -> None:
823885
"""Close the transport and clean up resources.
824886
@@ -859,6 +921,16 @@ async def close(self) -> None:
859921
await self._stderr_task.wait()
860922
self._stderr_task = None
861923

924+
# Cancel the process-exit watcher (same pattern as stderr task)
925+
if (
926+
self._exit_watcher_task is not None
927+
and not self._exit_watcher_task.done()
928+
):
929+
self._exit_watcher_task.cancel()
930+
with suppress(Exception):
931+
await self._exit_watcher_task.wait()
932+
self._exit_watcher_task = None
933+
862934
# Close stdin stream (hold the write lock to prevent a race with
863935
# concurrent writes). Bounded: a writer blocked on a full stdin
864936
# pipe must not pin the shielded scope forever.
@@ -978,7 +1050,19 @@ def guard(length: int) -> None:
9781050
)
9791051

9801052
try:
981-
async for chunk in self._stdout_stream:
1053+
stream_iter = aiter(self._stdout_stream)
1054+
while True:
1055+
# Flag the park-on-read state (and count chunks) so
1056+
# _unblock_reader_on_exit can tell an idle pipe apart from a
1057+
# reader suspended at `yield` by consumer backpressure.
1058+
self._awaiting_stdout = True
1059+
try:
1060+
chunk = await anext(stream_iter)
1061+
except StopAsyncIteration:
1062+
break
1063+
finally:
1064+
self._awaiting_stdout = False
1065+
self._stdout_chunks += 1
9821066
for line in framer.push(chunk):
9831067
guard(len(line))
9841068
data = _parse_stdout_line(line)
@@ -1006,9 +1090,15 @@ def guard(length: int) -> None:
10061090
if data is not None:
10071091
yield data
10081092

1009-
# Check process completion and handle errors
1093+
# Check process completion and handle errors. Prefer the already-set
1094+
# returncode: when _unblock_reader_on_exit closed the stream, wait()
1095+
# would block right here on the asyncio backend (it is gated on the
1096+
# same orphan-held pipe the watcher just detected — see that method).
10101097
try:
1011-
returncode = await self._process.wait()
1098+
if self._process.returncode is not None:
1099+
returncode = self._process.returncode
1100+
else:
1101+
returncode = await self._process.wait()
10121102
except Exception:
10131103
returncode = -1
10141104

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: 147 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2764,3 +2764,150 @@ def test_posix_allows_metacharacters(self):
27642764
cmd = transport._build_command()
27652765
assert "--resume=title & % | notes" in cmd
27662766
assert "--session-id=a>b" in cmd
2767+
=======
2768+
class TestReaderUnblockOnProcessExit:
2769+
"""Regression tests for #1110: process death must end read_messages even
2770+
when stdout never reaches EOF.
2771+
2772+
Helper processes spawned by the CLI inherit its stdout pipe; if the CLI
2773+
dies without reaping them, EOF never arrives. On the asyncio backend
2774+
``process.wait()`` is gated on the same pipes, so the mocks here enforce
2775+
the real contract: ``wait()`` never returns, only ``returncode`` is set.
2776+
"""
2777+
2778+
class _PipeHeldOpenStream:
2779+
"""Stdout stream whose EOF never arrives (orphan holds the pipe).
2780+
2781+
Yields the given chunks, then parks forever — until aclose(), after
2782+
which the parked (or next) read raises ClosedResourceError, exactly
2783+
like a real TextReceiveStream.
2784+
"""
2785+
2786+
def __init__(self, chunks: list[str]) -> None:
2787+
self._chunks = list(chunks)
2788+
self._closed = anyio.Event()
2789+
2790+
def __aiter__(self) -> "TestReaderUnblockOnProcessExit._PipeHeldOpenStream":
2791+
return self
2792+
2793+
async def __anext__(self) -> str:
2794+
if self._closed.is_set():
2795+
raise anyio.ClosedResourceError
2796+
if self._chunks:
2797+
return self._chunks.pop(0)
2798+
await self._closed.wait()
2799+
raise anyio.ClosedResourceError
2800+
2801+
async def aclose(self) -> None:
2802+
self._closed.set()
2803+
2804+
class _ExitedProcessWithHeldPipes:
2805+
"""Process that has exited, but whose wait() never completes.
2806+
2807+
Mirrors asyncio's behavior when an inherited pipe outlives the child:
2808+
returncode is set from the exit signal while wait() stays blocked on
2809+
pipe disconnection. The fix must not rely on wait().
2810+
"""
2811+
2812+
def __init__(self, returncode: int) -> None:
2813+
self.returncode = returncode
2814+
2815+
async def wait(self) -> int:
2816+
await anyio.sleep(3600)
2817+
return self.returncode
2818+
2819+
def _make_transport(self, returncode: int, chunks: list[str]):
2820+
transport = SubprocessCLITransport(prompt="test", options=make_options())
2821+
transport._process = self._ExitedProcessWithHeldPipes(returncode) # type: ignore[assignment]
2822+
transport._stdout_stream = self._PipeHeldOpenStream(chunks) # type: ignore[assignment]
2823+
return transport
2824+
2825+
def test_crash_with_orphan_held_pipe_raises_process_error(self, monkeypatch):
2826+
"""CLI died with exit 1, orphan holds stdout: must raise, not hang."""
2827+
from claude_agent_sdk._errors import ProcessError
2828+
from claude_agent_sdk._internal._task_compat import spawn_detached
2829+
from claude_agent_sdk._internal.transport import subprocess_cli
2830+
2831+
monkeypatch.setattr(subprocess_cli, "_EXIT_STREAM_GRACE_SECONDS", 0.05)
2832+
2833+
async def _test():
2834+
transport = self._make_transport(
2835+
returncode=1,
2836+
chunks=['{"type": "assistant", "message": {"content": []}}\n'],
2837+
)
2838+
watcher = spawn_detached(transport._unblock_reader_on_exit())
2839+
received = []
2840+
try:
2841+
with anyio.fail_after(5):
2842+
with pytest.raises(ProcessError, match="exit code 1"):
2843+
async for message in transport.read_messages():
2844+
received.append(message)
2845+
finally:
2846+
if not watcher.done():
2847+
watcher.cancel()
2848+
# Output produced before death must still be delivered.
2849+
assert len(received) == 1
2850+
assert received[0]["type"] == "assistant"
2851+
2852+
anyio.run(_test)
2853+
2854+
def test_clean_exit_with_orphan_held_pipe_ends_stream(self, monkeypatch):
2855+
"""CLI exited 0 but orphan holds stdout: stream must end cleanly."""
2856+
from claude_agent_sdk._internal._task_compat import spawn_detached
2857+
from claude_agent_sdk._internal.transport import subprocess_cli
2858+
2859+
monkeypatch.setattr(subprocess_cli, "_EXIT_STREAM_GRACE_SECONDS", 0.05)
2860+
2861+
async def _test():
2862+
transport = self._make_transport(
2863+
returncode=0,
2864+
chunks=['{"type": "result", "subtype": "success"}\n'],
2865+
)
2866+
watcher = spawn_detached(transport._unblock_reader_on_exit())
2867+
received = []
2868+
try:
2869+
with anyio.fail_after(5):
2870+
async for message in transport.read_messages():
2871+
received.append(message)
2872+
finally:
2873+
if not watcher.done():
2874+
watcher.cancel()
2875+
assert len(received) == 1
2876+
assert received[0]["type"] == "result"
2877+
2878+
anyio.run(_test)
2879+
2880+
def test_backpressure_is_not_mistaken_for_idle_pipe(self, monkeypatch):
2881+
"""A reader parked at yield (slow consumer) must not be force-closed.
2882+
2883+
The consumer takes longer than the grace window to process each
2884+
message while more chunks are still arriving; every chunk must be
2885+
delivered before the trailing ProcessError.
2886+
"""
2887+
from claude_agent_sdk._errors import ProcessError
2888+
from claude_agent_sdk._internal._task_compat import spawn_detached
2889+
from claude_agent_sdk._internal.transport import subprocess_cli
2890+
2891+
monkeypatch.setattr(subprocess_cli, "_EXIT_STREAM_GRACE_SECONDS", 0.05)
2892+
2893+
async def _test():
2894+
chunks = [
2895+
f'{{"type": "assistant", "message": {{"content": [], "i": {i}}}}}\n'
2896+
for i in range(5)
2897+
]
2898+
transport = self._make_transport(returncode=1, chunks=chunks)
2899+
watcher = spawn_detached(transport._unblock_reader_on_exit())
2900+
received = []
2901+
try:
2902+
with anyio.fail_after(10):
2903+
with pytest.raises(ProcessError, match="exit code 1"):
2904+
async for message in transport.read_messages():
2905+
received.append(message)
2906+
# Slower than the grace window.
2907+
await anyio.sleep(0.15)
2908+
finally:
2909+
if not watcher.done():
2910+
watcher.cancel()
2911+
assert len(received) == 5
2912+
2913+
anyio.run(_test)

0 commit comments

Comments
 (0)