Skip to content

Commit d97d137

Browse files
committed
fix(client): end the read stream when stdout hits EOF on a live server
`stdout_reader` closes the read stream inside `async with read_stream_writer`, but its `finally` runs `_drain_stdout` first, and before this change that drain sat in a shielded `while True: await process.stdout.receive()`. When the server closes stdout without exiting, the drain waits on the still-open pipes of a live process that will never write again, so the writer is never closed, the session never sees the end of the stream, and every waiting request waits forever. A crashed wrapper or a closed fd reproduces it without any mistake on the caller's side. Terminate the process before draining unless shutdown already asked for it. Shutdown keeps its own stop sequence, and the drain keeps its reason to exist (a server flushing its last output must not block on a full pipe), it just runs against a process that is now closing its pipes. That also makes the read side symmetric with `stdin_writer`, which already closes the read stream on a failed write so the session ends instead of hanging.
1 parent af4c6f4 commit d97d137

2 files changed

Lines changed: 33 additions & 4 deletions

File tree

‎src/mcp/client/stdio.py‎

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -136,7 +136,7 @@ async def stdio_client(
136136
read_stream_writer, read_stream = anyio.create_memory_object_stream[SessionMessage | Exception](0)
137137
write_stream, write_stream_reader = anyio.create_memory_object_stream[SessionMessage](0)
138138

139-
shutting_down = False
139+
shutting_down_event = anyio.Event()
140140
writer_done = anyio.Event()
141141

142142
async def stdout_reader() -> None:
@@ -157,13 +157,17 @@ async def stdout_reader() -> None:
157157
except (anyio.ClosedResourceError, anyio.BrokenResourceError):
158158
return # the session is gone; only the drain below remains
159159
finally:
160+
# The server is usually gone by now; if it is not, its pipes stay
161+
# open and the drain below would wait on it forever.
162+
if not shutting_down_event.is_set():
163+
await _terminate_process_tree(process)
160164
await _drain_stdout(process)
161165
except anyio.ClosedResourceError:
162166
pass # our own shutdown closed the stdout stream under the read
163167
except (anyio.BrokenResourceError, ConnectionError):
164168
# Teardown noise during shutdown, a real failure otherwise; either way
165169
# the session sees clean closure when the read stream closes.
166-
if not shutting_down:
170+
if not shutting_down_event.is_set():
167171
logger.exception("Reading from the MCP server's stdout failed mid-session")
168172

169173
async def stdin_writer() -> None:
@@ -204,7 +208,7 @@ async def shutdown() -> None:
204208
try:
205209
yield read_stream, write_stream
206210
finally:
207-
shutting_down = True
211+
shutting_down_event.set()
208212
# Shutdown must finish even under caller cancellation, or the server
209213
# process would leak; every wait inside is bounded. (Native
210214
# task.cancel() and the fallback's worker threads can still defeat it.)

‎tests/client/test_stdio.py‎

Lines changed: 26 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -323,6 +323,29 @@ async def test_invalid_json_from_the_server_surfaces_as_an_in_stream_exception(
323323
assert await _next_message(read_stream) == ping
324324

325325

326+
@pytest.mark.anyio
327+
async def test_stdout_eof_while_the_server_keeps_running_ends_the_read_stream(
328+
monkeypatch: pytest.MonkeyPatch,
329+
) -> None:
330+
"""EOF on stdout closes the session even though the process never exited.
331+
332+
A crashed wrapper or a closed fd leaves the server alive with its stdin and stderr
333+
still open, so nothing else ends the connection; the read stream must close before
334+
the reader parks in the drain, which waits on a live server's pipes forever.
335+
"""
336+
process = FakeProcess()
337+
terminated = install_fake_process(monkeypatch, process)
338+
339+
with anyio.fail_after(5):
340+
async with stdio_client(FAKE_PARAMS) as (read_stream, _):
341+
process.close_stdout()
342+
with pytest.raises(anyio.EndOfStream):
343+
await read_stream.receive()
344+
345+
# The server is still around, so it is killed instead of left behind.
346+
assert terminated == [process]
347+
348+
326349
@pytest.mark.anyio
327350
async def test_a_server_that_dies_before_responding_fails_initialize_with_connection_closed(
328351
monkeypatch: pytest.MonkeyPatch,
@@ -723,6 +746,8 @@ async def test_a_failed_write_to_a_live_server_closes_the_read_stream_instead_of
723746
1. A send fails with EIO while the server is alive; the read stream ends.
724747
2. Output the server produces afterwards is still drained, so it cannot wedge
725748
on a full pipe.
749+
3. The server outlived the session and gets the same termination shutdown would
750+
have given it.
726751
"""
727752
ping = JSONRPCRequest(jsonrpc="2.0", id=1, method="ping")
728753
pong = JSONRPCResponse(jsonrpc="2.0", id=1, result={})
@@ -744,7 +769,7 @@ async def test_a_failed_write_to_a_live_server_closes_the_read_stream_instead_of
744769
assert process.pending_stdout_chunks() == 0
745770

746771
assert process.written == []
747-
assert terminated == []
772+
assert terminated == [process]
748773

749774

750775
@pytest.mark.anyio

0 commit comments

Comments
 (0)