Don't close real process stdio in stdio_server (#1933)

src/mcp/server/stdio.py

@@ -17,9 +17,12 @@ async def run_server():
     ```
 """
 
+import io
+import os
 import sys
 from contextlib import asynccontextmanager
 from io import TextIOWrapper
+from typing import TextIO
 
 import anyio
 import anyio.lowlevel
@@ -34,14 +37,46 @@ async def stdio_server(stdin: anyio.AsyncFile[str] | None = None, stdout: anyio.
     """Server transport for stdio: this communicates with an MCP client by reading
     from the current process' stdin and writing to stdout.
     """
-    # Purposely not using context managers for these, as we don't want to close
-    # standard process handles. Encoding of stdin/stdout as text streams on
-    # python is platform-dependent (Windows is particularly problematic), so we
-    # re-wrap the underlying binary stream to ensure UTF-8.
+    # We don't want to close the process' standard handles. Wrapping
+    # sys.std{in,out}.buffer in a TextIOWrapper is not enough on its own: the
+    # wrapper's __del__ finalizer closes the buffer it wraps, so once the wrapper
+    # is garbage-collected the real sys.std{in,out} is closed and any later
+    # print()/read raises "ValueError: I/O operation on closed file." (#1933).
+    # Encoding of stdin/stdout as text streams on python is platform-dependent
+    # (Windows is particularly problematic), so we still re-wrap the underlying
+    # binary stream to ensure UTF-8.
+    #
+    # Preferred path: dup the underlying fd (os.dup is Windows-safe) and wrap the
+    # copy, so the wrapper only ever closes the duplicate; we close those wrappers
+    # on exit to free the dup'd fds. When sys.std* has no real fd (pytest capture,
+    # embedded interpreters, injected in-memory streams), we fall back to wrapping
+    # .buffer directly and detach() the wrapper on exit, which severs it from the
+    # buffer without closing it -- so the finalizer can no longer close the real
+    # handle either.
+    to_close: list[TextIOWrapper] = []
+    to_detach: list[TextIOWrapper] = []
+
+    def wrap_std(std: TextIO, mode: str, errors: str | None) -> anyio.AsyncFile[str]:
+        try:
+            binary = open(os.dup(std.fileno()), mode + "b", closefd=True)
+            wrapper = TextIOWrapper(binary, encoding="utf-8", errors=errors)
+            to_close.append(wrapper)
+        except (AttributeError, OSError, ValueError, io.UnsupportedOperation):
+            # No real fd. A bufferless in-memory text stream (e.g. io.StringIO)
+            # has no .buffer to re-wrap, so use it directly -- it is already text,
+            # and we did not create it, so there is nothing to tear down. Otherwise
+            # re-wrap .buffer and detach() on exit so the finalizer cannot close the
+            # real handle.
+            if not hasattr(std, "buffer"):
+                return anyio.wrap_file(std)
+            wrapper = TextIOWrapper(std.buffer, encoding="utf-8", errors=errors)
+            to_detach.append(wrapper)
+        return anyio.wrap_file(wrapper)
+
     if not stdin:
-        stdin = anyio.wrap_file(TextIOWrapper(sys.stdin.buffer, encoding="utf-8", errors="replace"))
+        stdin = wrap_std(sys.stdin, "r", errors="replace")
     if not stdout:
-        stdout = anyio.wrap_file(TextIOWrapper(sys.stdout.buffer, encoding="utf-8"))
+        stdout = wrap_std(sys.stdout, "w", errors=None)
 
     read_stream_writer, read_stream = create_context_streams[SessionMessage | Exception](0)
     write_stream, write_stream_reader = create_context_streams[SessionMessage](0)
@@ -71,7 +106,17 @@ async def stdout_writer():
         except anyio.ClosedResourceError:  # pragma: no cover
             await anyio.lowlevel.checkpoint()
 
-    async with anyio.create_task_group() as tg:
-        tg.start_soon(stdin_reader)
-        tg.start_soon(stdout_writer)
-        yield read_stream, write_stream
+    try:
+        async with anyio.create_task_group() as tg:
+            tg.start_soon(stdin_reader)
+            tg.start_soon(stdout_writer)
+            yield read_stream, write_stream
+    finally:
+        # Close the wrappers around dup'd fds (frees the duplicate) and detach the
+        # wrappers around the real .buffer (severs them without closing the real
+        # handle, so their finalizers can't close it either). Neither touches the
+        # process' standard handles or a caller-injected stream.
+        for wrapper in to_close:
+            wrapper.close()
+        for wrapper in to_detach:
+            wrapper.detach()

tests/server/test_stdio.py

@@ -1,5 +1,7 @@
+import gc
 import io
 import sys
+import tempfile
 import threading
 from collections.abc import AsyncIterator
 from contextlib import asynccontextmanager
@@ -75,6 +77,68 @@ async def test_stdio_server_round_trips_messages_over_injected_streams() -> None
     assert received_responses[1] == JSONRPCResponse(jsonrpc="2.0", id=4, result={})
 
 
+@pytest.mark.anyio
+async def test_stdio_server_does_not_close_real_std_handles(monkeypatch: pytest.MonkeyPatch) -> None:
+    """Regression test for issue #1933: the default path must not close process stdio.
+
+    `stdio_server()` re-wraps `sys.stdin.buffer`/`sys.stdout.buffer` in a `TextIOWrapper`
+    to force UTF-8. The wrapper's `__del__` finalizer used to close the underlying buffer,
+    so any code running after the server exits raised
+    `ValueError: I/O operation on closed file.` on `print()` / stdout writes.
+
+    The default path dups the fds and wraps the copies, so tearing the wrappers down
+    closes only the duplicates and leaves the real process handles usable. Real
+    fd-backed temp files stand in for the process handles here so that the fix's
+    `sys.std{in,out}.fileno()` path is exercised; the empty stdin file reaches EOF
+    immediately so the reader worker unwinds without a client.
+    """
+    with (
+        tempfile.TemporaryFile(mode="rb", buffering=0) as stdin_raw,
+        tempfile.TemporaryFile(mode="wb", buffering=0) as stdout_raw,
+    ):
+        stdin_file = TextIOWrapper(stdin_raw, encoding="utf-8")  # empty -> immediate EOF
+        stdout_file = TextIOWrapper(stdout_raw, encoding="utf-8")
+        monkeypatch.setattr(sys, "stdin", stdin_file)
+        monkeypatch.setattr(sys, "stdout", stdout_file)
+
+        with anyio.fail_after(5):
+            async with stdio_server() as (read_stream, write_stream):
+                # Close the streams so the reader/writer tasks unwind and the
+                # server context exits (explicit aclose keeps this branch-free
+                # for the repo's 100% coverage gate).
+                await read_stream.aclose()
+                await write_stream.aclose()
+
+        # Force finalization of any TextIOWrapper the server created internally.
+        gc.collect()
+
+        # On the buggy code these assertions fail: the internal wrapper's __del__
+        # closed the buffer we handed in.
+        assert not stdin_raw.closed, "stdio_server closed the real stdin buffer"
+        assert not stdout_raw.closed, "stdio_server closed the real stdout buffer"
+        # And the process handles stay usable after the server exits.
+        stdout_file.write("after-server-exit\n")
+        stdout_file.flush()
+
+
+@pytest.mark.anyio
+async def test_stdio_server_bufferless_text_streams(monkeypatch: pytest.MonkeyPatch) -> None:
+    """Regression: default startup must not crash on bufferless in-memory text streams.
+
+    In-memory text streams like `io.StringIO` have neither a real `fileno()` nor a
+    `.buffer`, so both the fd-dup path and the `.buffer` re-wrap path fail. In that
+    case `stdio_server()` must use the stream directly. Before the fix, the fallback
+    reached for `std.buffer` and raised `AttributeError` during startup.
+    """
+    monkeypatch.setattr(sys, "stdin", io.StringIO())  # empty -> immediate EOF
+    monkeypatch.setattr(sys, "stdout", io.StringIO())
+
+    with anyio.fail_after(5):
+        async with stdio_server() as (read_stream, write_stream):
+            await read_stream.aclose()
+            await write_stream.aclose()
+
+
 @pytest.mark.anyio
 async def test_stdio_server_invalid_utf8(monkeypatch: pytest.MonkeyPatch) -> None:
     """Non-UTF-8 stdin bytes surface as an in-stream exception without killing the stream.