fix(stdio): use os.dup to avoid closing real stdin/stdout on server exit#2643
Closed
jrlprost wants to merge 1 commit into
Closed
fix(stdio): use os.dup to avoid closing real stdin/stdout on server exit#2643jrlprost wants to merge 1 commit into
jrlprost wants to merge 1 commit into
Conversation
TextIOWrapper wrapping sys.stdin.buffer closes the underlying buffer when the wrapper is garbage-collected. After stdio_server() exits, this made any subsequent print()/sys.stdin.read() raise ValueError on the closed fd. Duplicate the file descriptor with os.dup() so our TextIOWrapper owns a private copy; closing it leaves the original process stdin/stdout intact. Falls back to the original buffer when the stream has no real fd (e.g. BytesIO in tests), matching existing behaviour in that case. Fixes modelcontextprotocol#1933.
Contributor
|
closing as there's already an existing PR for this issue |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #1933.
Root cause
stdio_server()wrapssys.stdin.bufferin aTextIOWrapper. When that wrapper is garbage-collected after the context manager exits, Python's finalizer callsclose()on it, which propagates down and closessys.stdin.buffer(fd 0). Any subsequentprint()orsys.stdin.read()then raisesValueError: I/O operation on closed file.The existing comment ("Purposely not using context managers for these, as we don't want to close standard process handles") shows the intent was correct, but the wrapper's
__del__closes the buffer regardless.Fix
Duplicate the file descriptor with
os.dup()before wrapping. OurTextIOWrappernow owns a private copy of the fd; closing it leaves the originalsys.stdin/sys.stdoutuntouched.Falls back to the original buffer when the stream is not backed by a real fd (e.g.
BytesIOin tests), matching prior behaviour in that case. The dup'd wrappers are explicitly closed in afinallyblock after the task group exits.Changes
src/mcp/server/stdio.py:os.dup()the fd; track ownership withstdin_created/stdout_created; close infinally.tests/server/test_stdio.py: regression test using real OS pipes — assertssys.stdin.bufferandsys.stdout.bufferare still open afterstdio_server()exits.Testing
Branch coverage for
src/mcp/server/stdio.py: 100 %.The
except io.UnsupportedOperationbranch is covered by the existingtest_stdio_server_invalid_utf8test (BytesIO stdin). The new test covers thetrysuccess path with real pipe fds.