Skip to content

Commit cbac04d

Browse files
committed
Never close a buffer the transport does not own
The transport's text layers now detach instead of closing on garbage collection. In the in-place paths the wrapped buffer is the sys stream's own, and closing it destroyed sys.stdout for the rest of the process (the issue #1933 class, which the claimed path had already fixed incidentally via the private descriptor). Also strengthens the fd 0 watchdog migration note (the null device reports permanently readable, so any-event watchers misfire at startup rather than merely never firing) and restores the note about child output volume flowing into the client's stderr channel.
1 parent 3e5fa68 commit cbac04d

3 files changed

Lines changed: 59 additions & 8 deletions

File tree

docs/migration.md

Lines changed: 15 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1864,12 +1864,21 @@ handler code can no longer read protocol bytes or write into the stream (the
18641864
servers have nothing to do, and code that inspects or manipulates fd 0/1 directly
18651865
during a session now sees the diversions, not the wire.
18661866

1867-
One pattern needs migrating: a watchdog thread that `poll()`s fd 0 for `POLLHUP` to
1868-
detect a vanished client will no longer fire, because the null device never reports
1869-
it (a POSIX-specific pattern; `select.poll` does not exist on Windows). Watch the
1870-
parent process instead: on POSIX, exit when `os.getppid()` changes, which happens
1871-
when the client dies because orphaned processes are reparented. That works on both
1872-
v1 and v2 and does not depend on descriptor layout.
1867+
One pattern needs migrating: watchdog threads that watch fd 0 to detect a vanished
1868+
client (a POSIX-specific pattern; `select.poll` does not exist on Windows). The null
1869+
device does not behave like the old pipe: it never reports `POLLHUP`, and it reports
1870+
readable immediately and permanently (`POLLIN|POLLERR` from `poll()` on Linux, ready
1871+
from `select()`, and macOS can report `POLLNVAL` for devices). A watcher waiting for
1872+
`POLLHUP` is silently disarmed; a watcher that treats any event as "client gone" now
1873+
fires at startup instead of never. Watch the parent process instead: on POSIX, exit
1874+
when `os.getppid()` changes, which happens when the client dies because orphaned
1875+
processes are reparented. That works on both v1 and v2 and does not depend on
1876+
descriptor layout.
1877+
1878+
Also worth knowing: a child process that streams large output to its inherited
1879+
stdout now streams it into the client's stderr channel. Capture output you do not
1880+
want in the client's logs, and be aware that a client which never drains its stderr
1881+
pipe applies back-pressure to the server (true of stderr logging on v1 as well).
18731882

18741883
### WebSocket transport removed
18751884

src/mcp/server/stdio.py

Lines changed: 15 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -49,6 +49,19 @@ class _StreamClaim:
4949
private_fd: int | None = None
5050

5151

52+
class _UnownedTextWrapper(TextIOWrapper):
53+
"""Text layer whose close never closes the underlying buffer.
54+
55+
The buffer is not the transport's to close: in the in-place paths it is the
56+
sys stream's own buffer, and closing it at garbage collection destroyed
57+
sys.stdout for the rest of the process (issue #1933).
58+
"""
59+
60+
def close(self) -> None:
61+
with suppress(ValueError):
62+
self.detach()
63+
64+
5265
def _is_backed_by_fd(stream: TextIO, fd: int) -> bool:
5366
try:
5467
return stream.buffer.fileno() == fd
@@ -158,10 +171,10 @@ async def stdio_server(stdin: anyio.AsyncFile[str] | None = None, stdout: anyio.
158171
try:
159172
if not stdin:
160173
stdin_buffer, restore_stdin = _claim_fd(0, sys.stdin, "rb", _open_stdin_diversion)
161-
stdin = anyio.wrap_file(TextIOWrapper(stdin_buffer, encoding="utf-8", errors="replace"))
174+
stdin = anyio.wrap_file(_UnownedTextWrapper(stdin_buffer, encoding="utf-8", errors="replace"))
162175
if not stdout:
163176
stdout_buffer, restore_stdout = _claim_fd(1, sys.stdout, "wb", _open_stdout_diversion)
164-
stdout = anyio.wrap_file(TextIOWrapper(stdout_buffer, encoding="utf-8"))
177+
stdout = anyio.wrap_file(_UnownedTextWrapper(stdout_buffer, encoding="utf-8"))
165178

166179
read_stream_writer, read_stream = create_context_streams[SessionMessage | Exception](0)
167180
write_stream, write_stream_reader = create_context_streams[SessionMessage](0)

tests/server/test_stdio.py

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1+
import gc
12
import io
23
import os
34
import sys
@@ -502,6 +503,34 @@ def failing_diversion() -> int:
502503
await write_stream.aclose()
503504

504505

506+
@pytest.mark.anyio
507+
async def test_a_degraded_session_does_not_close_the_sys_stream_it_served(
508+
monkeypatch: pytest.MonkeyPatch,
509+
) -> None:
510+
"""The transport's text layer never closes a buffer it does not own.
511+
512+
Regression for the issue #1933 class: in the in-place paths the transport wraps
513+
the sys stream's own buffer, and wrapper garbage collection must not close it.
514+
"""
515+
with _pipe_planted_on_fd0(monkeypatch) as (_, in_w):
516+
os.close(in_w)
517+
monkeypatch.setattr(sys, "stdout", TextIOWrapper(io.BytesIO(), encoding="utf-8"))
518+
519+
def failing_dup_above_std(fd: int) -> int:
520+
raise OSError("forced degrade")
521+
522+
monkeypatch.setattr("mcp.server.stdio._dup_above_std", failing_dup_above_std)
523+
524+
with anyio.fail_after(5):
525+
async with stdio_server() as (read_stream, write_stream):
526+
read_stream.close()
527+
await write_stream.aclose()
528+
529+
gc.collect()
530+
assert not sys.stdin.buffer.closed
531+
assert not sys.stdout.buffer.closed
532+
533+
505534
class _GatedStdin(io.RawIOBase):
506535
"""Raw stdin double: serves its frames, then blocks until released before EOF.
507536

0 commit comments

Comments
 (0)