Fix stdio_client BrokenResourceError race condition during shutdown#2268
Fix stdio_client BrokenResourceError race condition during shutdown#2268weiguangli-io wants to merge 1 commit intomodelcontextprotocol:mainfrom
Conversation
During shutdown, the finally block closes the receiving end of the read_stream memory channel while stdout_reader may still be blocked on send(). This causes BrokenResourceError (not ClosedResourceError) because the *other* end of the stream was closed. The existing except clause only caught ClosedResourceError, letting BrokenResourceError propagate into an ExceptionGroup. Catch BrokenResourceError alongside ClosedResourceError in both stdout_reader and stdin_writer to handle this race gracefully. Closes modelcontextprotocol#1960 Github-Issue:modelcontextprotocol#1960 Reported-by:maxisbey
Bortlesboat
left a comment
Bortlesboat
left a comment
There was a problem hiding this comment.
ACK. Clean minimal fix — BrokenResourceError is the correct counterpart to catch when the receiving end closes under a pending send().
| session_message = SessionMessage(message) | ||
| await read_stream_writer.send(session_message) | ||
| except anyio.ClosedResourceError: # pragma: lax no cover | ||
| except (anyio.ClosedResourceError, anyio.BrokenResourceError): # pragma: lax no cover |
There was a problem hiding this comment.
Noting for future readers: the SSE client (sse.py) avoids this class of race by calling tg.cancel_scope.cancel() in its finally block, which cancels tasks before closing their streams. The stdio client takes a different approach — letting stream closure propagate to tasks and catching the resulting exceptions. Both are valid; this PR correctly handles the full set of exceptions that anyio can raise in this scenario.
|
This would also resolve the semaphore leak reported in openai/openai-agents-python#618 — the SDK currently works around it by pre-closing the write stream before exit stack teardown (openai/openai-agents-python#2802), but that reaches into Happy to help test if useful. |
3 participants
Summary
Fixes #1960. Supersedes #2219 (which was closed).
During
stdio_clientshutdown, thefinallyblock closesread_stream(the receiving end of a zero-buffer memory channel) whilestdout_readermay still be blocked onsend(). When the receiving end closes underneath a pendingsend(), anyio raisesBrokenResourceError— notClosedResourceError. The existingexceptclause only caughtClosedResourceError, soBrokenResourceErrorescaped into anExceptionGroup.The fix catches
BrokenResourceErroralongsideClosedResourceErrorin bothstdout_readerandstdin_writer. This is the minimal, correct change:ClosedResourceErrormeans "you called send on a handle you already closed", whileBrokenResourceErrormeans "the other end was closed" — both are expected during shutdown and should be handled identically.Test plan
test_stdio_client_no_broken_resource_error_on_shutdownthat reproduces the exact scenario: server sends a JSON-RPC message, client exits without consuming the read streamtests/client/test_stdio.pytests pass