Skip to content

Close a refused SSE stream outside the registry lock - #592

Open
koic wants to merge 1 commit into
modelcontextprotocol:mainfrom
koic:close_a_refused_sse_stream_outside_the_registry_lock
Open

koic wants to merge 1 commit into
modelcontextprotocol:mainfrom
koic:close_a_refused_sse_stream_outside_the_registry_lock

Conversation

@koic

@koic koic commented Oct 9, 2026

Copy link
Copy Markdown
Member

Motivation and Context

StreamableHTTPTransport closes every stream outside its registry lock, so a close that blocks on the peer cannot hold every other session on @mutex. One path broke that rule: when a GET stream could not be registered, because the session had gone or another stream had taken its place, store_stream_for_session closed the refused stream inside the lock, and an error raised by that close escaped the Rack body.

The refused stream is now closed after the lock is released and through close_stream_safely, like the streams the transport drops everywhere else.

How Has This Been Tested?

Three new tests in test/mcp/server/transports/streamable_http_transport_test.rb: two refuse a second stream for a session, one checking from inside the stream's close that the lock is free, the other that a close raising IOError is swallowed and the session keeps serving, and the third refuses a stream for a session that is gone, checking the lock the same way. Against the previous library, the first and the third find the lock held and the second raises.

Breaking Changes

None.

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Documentation update

Checklist

  • I have read the MCP Documentation
  • My code follows the repository's style guidelines
  • New and existing tests pass locally
  • I have added appropriate error handling
  • I have added or updated documentation as needed

## Motivation and Context

`StreamableHTTPTransport` closes every stream outside its registry lock, so a close that blocks
on the peer cannot hold every other session on `@mutex`. One path broke that rule: when a `GET` stream
could not be registered, because the session had gone or another stream had taken its place,
`store_stream_for_session` closed the refused stream inside the lock, and an error raised by
that close escaped the Rack body.

The refused stream is now closed after the lock is released and through `close_stream_safely`,
like the streams the transport drops everywhere else.

## How Has This Been Tested?

Three new tests in `test/mcp/server/transports/streamable_http_transport_test.rb`:
two refuse a second stream for a session, one checking from inside the stream's `close` that
the lock is free, the other that a `close` raising `IOError` is swallowed and the session keeps serving,
and the third refuses a stream for a session that is gone, checking the lock the same way.
Against the previous library, the first and the third find the lock held and the second raises.

## Breaking Changes

None.

This branch has not been deployed

No deployments
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.

2 participants