Skip to content

fix: use os.dup to avoid closing real stdin/stdout in stdio_server#2737

Open
xlyoung wants to merge 1 commit into
modelcontextprotocol:mainfrom
xlyoung:fix/stdio-close-real-stdin-stdout
Open

fix: use os.dup to avoid closing real stdin/stdout in stdio_server#2737
xlyoung wants to merge 1 commit into
modelcontextprotocol:mainfrom
xlyoung:fix/stdio-close-real-stdin-stdout

Conversation

@xlyoung
Copy link
Copy Markdown

@xlyoung xlyoung commented May 31, 2026

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

  • All existing tests pass
  • Added new test test_stdio_server_uses_os_dup to verify the fix
  • Tested with -race flag

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).
Copy link
Copy Markdown

@StantonMatt StantonMatt left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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...HEAD

The 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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Using transport="stdio" closes real stdio, causing ValueError after server exits

2 participants