fix(client/stdio): fall back to os.devnull when sys.stderr is None#2551
Open
namor5772 wants to merge 1 commit intomodelcontextprotocol:mainfrom
Open
fix(client/stdio): fall back to os.devnull when sys.stderr is None#2551namor5772 wants to merge 1 commit intomodelcontextprotocol:mainfrom
namor5772 wants to merge 1 commit intomodelcontextprotocol:mainfrom
Conversation
Under pythonw.exe on Windows (desktop shortcuts, silent .bat launches, anything without a console attached), sys.stderr is None. The previous default `errlog: TextIO = sys.stderr` then propagated that None handle into the subprocess as its stderr. asyncio's Windows ProactorEventLoop subprocess transport mishandles a None stderr handle, corrupting IOCP routing on the read pump. The symptom is `ClosedResourceError` on the first real RPC after `initialize()` succeeds. Bursty servers like `@modelcontextprotocol/server-filesystem` (whose initialize → list_tools traffic happens within milliseconds) hit this consistently; quieter servers like Gmail clients usually slip through. Reproducing requires launching a stdio MCP client via pythonw.exe with no stderr redirect — running the same code from a console (python.exe) or with stderr redirected to a file works because sys.stderr is then a real handle. Fix: change `errlog: TextIO = sys.stderr` to `errlog: TextIO | None = None` and resolve at call time. Falls back to `sys.stderr` when it is a real handle, `os.devnull` otherwise. Backward compatible — callers passing an explicit `errlog` keep the same behavior. Also applied the same fallback in `_create_platform_compatible_process` as defense in depth, so any internal caller that omits `errlog` is safe regardless of whether `sys.stderr` is None. Verified by reproducing the bug under pythonw.exe (filesystem-server list_tools fails with ClosedResourceError) and confirming the fix restores it to working state without changing behavior under python.exe or with explicit errlog.
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.
Summary
Make
stdio_client(and the internal_create_platform_compatible_process) toleratesys.stderr is None, which is the case underpythonw.exeon Windows — i.e. every desktop shortcut, every silent.batlaunch, and anything else launched without a console attached.Motivation
When a stdio MCP client is hosted in a Python process whose
sys.stderrisNone, the current defaulterrlog: TextIO = sys.stderrpropagates thatNoneinto the spawned subprocess as its stderr handle. asyncio's Windows ProactorEventLoop subprocess transport mishandles aNonestderr — the symptom isClosedResourceErroron the first real RPC afterinitialize()succeeds.Bursty servers like
@modelcontextprotocol/server-filesystem(whoseinitialize→list_toolstraffic happens within milliseconds) hit this consistently. Quieter servers (e.g. typical Gmail clients) usually slip through with the same broken setup, which made this bug elusive — the affected client developers see "filesystem-server is broken in my app but Gmail works fine."The triggering condition is purely the launch context, not the application logic:
sys.stderrpython.exe MyApp.py(console)pythonw.exe MyApp.py 2> log.txt(redirected)pythonw.exe MyApp.py(no console, no redirect)pythonw.exe MyApp.pyRepro
A complete repro requires a Windows host plus pythonw.exe. From within a Tk/non-console Python app launched via
pythonw.exe MyApp.py(no stderr redirect), connect to@modelcontextprotocol/server-filesystemviastdio_client(StdioServerParameters(command=\"node\", args=[\"index.js\", path])), callawait session.initialize(), thenawait session.list_tools(). The list_tools call fails withanyio.ClosedResourceError. Switching the host topython.exe(console) makes the same code work without any other change.Fix
Change the public default from
errlog: TextIO = sys.stderrtoerrlog: TextIO | None = None, and resolve at call time:Same fallback applied in
_create_platform_compatible_processas defense in depth, so any internal call path that omitserrlogis safe regardless ofsys.stderrstate.Compatibility
errlog: unchanged behavior. The runtime check only fires whenerrlog is None.stdio_client(server)): now safe under pythonw, identical under python.exe (sincesys.stderris a real handle in that case).TextIOtoTextIO | None. Existing typed callers passingTextIOcontinue to satisfy the new signature; mypy/pyright won't break.Test plan
ClosedResourceErroron list_tools after initialize, against@modelcontextprotocol/server-filesystem).errlog=....sys.stderr = Noneand assertsstdio_clientdoes not raise (suggested follow-up; not in this PR to keep the diff minimal).Discovered while integrating the SDK into a tkinter desktop agent launched via Windows shortcut.