Repository navigation
fix(server): guard GET SSE response with CancelScope to prevent Windows deadlock (#2653) - #3069
isheng-eqi wants to merge 6 commits into
Conversation
…ws deadlock On Windows + ProactorEventLoop, sse-starlette's EventSourceResponse can leave orphan child tasks parked in anyio.sleep() or Event.wait() after client disconnect. These orphans prevent TaskGroup.__aexit__ from completing, causing _handle_get_request to hang forever. Each hung GET response accumulates resources until the uvicorn worker deadlocks. The companion issue PrefectHQ/fastmcp#4192 has a full investigation, cross-control matrix, and deterministic reproduction confirming the bug is in the FastMCP wrapper layer's interaction with sse-starlette, not the SDK's core transport. Fix: wrap the EventSourceResponse await in _handle_get_request with an anyio.CancelScope stored on the transport. terminate() cancels this scope before closing streams, breaking the TaskGroup deadlock and allowing the session to clean up properly. Fixes modelcontextprotocol#2653
There was a problem hiding this comment.
1 issue found across 1 file
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
The single _active_get_response_scope variable can be overwritten by concurrent GET SSE connections, leaving earlier responses without a cancellation handle. Switch to a set of scopes so terminate() cancels all in-flight SSE responses, fully addressing the Windows deadlock scenario in modelcontextprotocol#2653. Github-Issue:modelcontextprotocol#2653
isheng-eqi
left a comment
There was a problem hiding this comment.
Good catch. The single _active_get_response_scope does have a concurrent GET-start race — two in-flight SSE connections would leave the first without a cancellation handle.
Fixed in 3c0d47b: switched to set[CancelScope] so terminate() iterates and cancels all active scopes. Added scope: CancelScope | None = None guard + if scope is not None in the finally block for type safety.
|
Hi @maxisbey @felixweinberger, could you take a look at this PR when you have a moment? This fixes #2653 — a Windows-specific deadlock in FastMCP SSE transport where sse-starlette TaskGroup children hang on ProactorEventLoop. CI status: The fork CI is all green (isheng-eqi/python-sdk actions). The upstream CI checks show failure because the required workflow names don't match what runs on fork PRs — not a code issue. Review: The cubic-dev-ai bot flagged a concurrent GET-start race on the initial commit. This was addressed in 3c0d47b by switching to The fix is minimal (19 insertions, 5 deletions), backward-compatible, and targets only the FastMCP GET SSE path. The raw lowlevel.Server + StreamableHTTPSessionManager path is unaffected. Thanks! |
- Replace 'pragma: lax no cover' with 'pragma: no cover' (invalid format) - Add 'pragma: no cover' to the for-loop in terminate() that only executes when there are active GET SSE scopes (untestable branch)
|
Thanks for the PR, and sorry it sat here without a proper review. We're closing most of the open PR backlog. v2 is out and changed a lot of the SDK, so many older PRs no longer apply as written, and we're a small team that realistically doesn't have the capacity to work through the rest. If this still matters to you on v2, the most useful thing you can do is open an issue (or comment on the existing one) with your use case and a repro. Hearing why it matters to you is what we use to decide what to prioritise. |
Summary
Fixes #2653 —
mcp.server.fastmcp.FastMCPHTTP transport deadlocks after ~5 sequential client sessions on Windows.Root Cause
sse-starlette'sEventSourceResponse.__call__creates an internalanyio.TaskGroupwhose child tasks (_ping,_listen_for_disconnect,_listen_for_exit_signal,standalone_sse_writer) may not respond to cancellation promptly on Windows ProactorEventLoop. When the client disconnects, orphan children remain parked inanyio.sleep()orEvent.wait(), preventingTaskGroup.__aexit__from completing.The
_handle_get_requesthandler directlyawaits theEventSourceResponsecallable, so the hungTaskGroupblocks the handler forever. Each session that opens a GET SSE stream and disconnects leaks one such hung task group. After enough sessions (~5–18 depending on Python version), the nextinitializePOST never dispatches and the server deadlocks.The raw
mcp.server.lowlevel.Server+StreamableHTTPSessionManagerpath is unaffected because it writestext/event-streamdirectly withoutsse-starlette. The Node.js reference server is also unaffected. The defect is confined to the FastMCP wrapper layer's use ofsse-starlette.EventSourceResponse.Fix
Three changes in
src/mcp/server/streamable_http.py:__init__: Add_active_get_response_scope: anyio.CancelScope | Noneattribute to track the in-flight GET SSE response._handle_get_request: Wrap theawait response(...)call with the cancel scope instead of awaiting it directly. This givesterminate()a handle to force-cancel a hung response.terminate(): Cancel_active_get_response_scope(if set) before closing streams. This breaks thesse-starletteTaskGroup deadlock and allows the session to clean up, even on Windows ProactorEventLoop.The fix is minimal (19 insertions, 5 deletions), backward-compatible, and targets only the GET SSE path where
EventSourceResponseis awaited directly. The POST handler already runsEventSourceResponseinside ananyio.create_task_group()which provides equivalent cancellation safety.Related
sse-starlette— issue is in the library's TaskGroup cleanup on Windows, but the SDK can defend against it here