fix: use os.dup to avoid closing real stdin/stdout in stdio_server - #2737
fix: use os.dup to avoid closing real stdin/stdout in stdio_server#2737xlyoung wants to merge 1 commit into
Conversation
Sorry, something went wrong.
Fixes modelcontextprotocol#1933 When stdio_server() wraps sys.stdin.buffer/sys.stdout.buffer directly, closing the context manager also closes the real file descriptors, causing ValueError on subsequent I/O operations. Fix: Use os.dup() to duplicate file descriptors before wrapping them. This ensures closing the wrappers doesn't affect the real stdin/stdout. Added fallback for environments where fileno() is not available (e.g., BytesIO-backed streams in tests).
StantonMatt
left a comment
There was a problem hiding this comment.
I checked 193c4f5 locally on Windows/Python 3.13.13:
uv run --frozen pytest tests/server/test_stdio.py -q
# 3 passed
git diff --check origin/main...HEADThe one visible red job is failing in tests/shared/test_sse.py::test_sse_session_cleanup_on_disconnect on Python 3.14 locked Ubuntu, not in the stdio tests.
Two branch-specific things stood out:
- uv.lock is rewritten even though this PR does not change dependencies.
- The new test_stdio_server_uses_os_dup only inspects source text for os.dup / os.fdopen, so it could pass without proving the original process stdio wrappers remain usable after stdio_server() exits.
This also overlaps with #2734, which targets the same #1933 path with behavior-level stdio assertions and currently has a green matrix. I would either drop the lockfile churn and switch this test to behavior, or close this in favor of #2734 if that branch is the intended fix.
Sorry, something went wrong.
|
Thanks for the PR. Tracking this in #2040 instead. I'm closing this out as part of a backlog cleanup around the v2 release. Feel free to reopen if this is still relevant. |
Sorry, something went wrong.
Fixes #1933
Problem
When stdio_server() wraps sys.stdin.buffer/sys.stdout.buffer directly, closing the context manager also closes the real file descriptors, causing ValueError: I/O operation on closed file on subsequent I/O operations.
Fix
Use os.dup() to duplicate file descriptors before wrapping them. This ensures closing the wrappers does not affect the real stdin/stdout.
Added fallback for environments where fileno() is not available (e.g., BytesIO-backed streams in tests).
Testing