Skip to content

[v1.x] fix: handle ClosedResourceError when transport closes mid-request#2334

Open
owendevereaux wants to merge 1 commit intomodelcontextprotocol:v1.xfrom
owendevereaux:fix/v1x-closed-resource-error-2328
Open

[v1.x] fix: handle ClosedResourceError when transport closes mid-request#2334
owendevereaux wants to merge 1 commit intomodelcontextprotocol:v1.xfrom
owendevereaux:fix/v1x-closed-resource-error-2328

Conversation

@owendevereaux
Copy link

Summary

Backports fixes from #2306 to v1.x to address issue #2328.

When the transport closes while handlers are processing requests (e.g., stdin EOF during a long-running tool call, or a malicious client sending invalid UTF-8 bytes that crashes the transport), the server could crash with ClosedResourceError when trying to send a response through the already-closed write stream.

Changes

  1. Wrap message loop in try/finally with cancel - When the transport closes, in-flight handlers are now cancelled instead of being left to attempt responding through a closed stream
  2. Catch ClosedResourceError on respond - If a response attempt fails due to closed transport, log and return gracefully instead of crashing
  3. Fix cancellation handling - Distinguish between client-initiated cancellation (already responded) and transport-close cancellation (should re-raise)
  4. Fix dict iteration in finally - Use list() snapshot when iterating _response_streams to avoid 'dictionary changed size during iteration' errors

Testing

Added two new test cases that reproduce the crash scenarios:

  • test_server_cancels_in_flight_handlers_on_transport_close
  • test_server_handles_transport_close_with_pending_server_to_client_requests

All existing tests continue to pass.

Fixes #2328

Backports fixes from modelcontextprotocol#2306 to v1.x to address issue modelcontextprotocol#2328.

When the transport closes while handlers are processing requests (e.g., stdin
EOF during a long-running tool call), the server could crash with
ClosedResourceError when trying to send a response through the already-closed
write stream.

This fix:
1. Wraps the message loop in a try/finally that cancels in-flight handlers
   when the transport closes, preventing them from attempting to respond
2. Catches BrokenResourceError and ClosedResourceError when calling
   message.respond() and logs instead of crashing
3. Properly re-raises transport-close cancellation to let the task group
   handle it (vs client-initiated cancellation which already sent a response)
4. Uses list() snapshot when iterating _response_streams in the finally block
   to avoid 'dictionary changed size during iteration' errors

Fixes modelcontextprotocol#2328
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.

1 participant