Skip to content

Bound the listen teardown on transport close - #595

Merged
koic merged 1 commit into
modelcontextprotocol:mainfrom
koic:bound_the_listen_teardown_on_transport_close
Oct 12, 2026
Merged

koic merged 1 commit into
modelcontextprotocol:mainfrom
koic:bound_the_listen_teardown_on_transport_close

Conversation

@koic

@koic koic commented Oct 11, 2026 •

Copy link
Copy Markdown
Member

Motivation and Context

StreamableHTTPTransport#close writes each open subscriptions/listen stream its SubscriptionsListenResult before closing it, under the stream's write mutex. Both the write and the wait for that mutex are unbounded: a client that stopped reading leaves its socket buffers full, so the result write blocks for as long as the connection lasts, and so does a delivery already holding the mutex, with close waiting behind it. A single such client kept the transport from closing at all.

The results are now written on a helper thread that close waits for at most LISTEN_TEARDOWN_TIMEOUT (5 seconds) in all. When the time is up, every stream still open is closed from close itself: on a Ruby IO that ends a write stalled on it with an IOError the helper rescues, while a stream object of another kind may keep the helper blocked, which no longer holds close. Either way that stream goes without its result, like one whose acknowledgement was never written, so the client re-sends. The helper is left to finish rather than killed, so a write mutex is never dropped in the middle of a write. An error other than a lost peer raised by a stream's write is reported through the exception reporter and no longer escapes close, which would have skipped the remaining streams and the session cleanup. Nor does an exception reporter that raises on that report: it is dropped, and the teardown goes on. Whichever of the helper and close takes a stream out of the pending set closes it, so a stream is closed once in either order.

How Has This Been Tested?

Three new tests in test/mcp/server/transports/streamable_http_transport_test.rb: the teardown returns within its timeout behind a held write mutex and behind a stalled result write, closing the stream and letting the helper end on its own, and a result write failing with an unexpected error is reported while the other streams still get their result. Against the previous library the third raises out of close, and a probe holding a stream's write mutex while calling close never returns, where it returns after 5 seconds now. A fourth test pairs the failing write with an exception reporter that raises: close still returns and tears the rest down, where the previous library raised out of it. The held-mutex and stalled-write tests count one close of their stream.

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

@koic
koic force-pushed the bound_the_listen_teardown_on_transport_close branch 2 times, most recently from 9944ae6 to eb8d3d2 Compare October 11, 2026 16:26
## Motivation and Context

`StreamableHTTPTransport#close` writes each open `subscriptions/listen` stream its `SubscriptionsListenResult`
before closing it, under the stream's write mutex. Both the write and the wait for that mutex are unbounded:
a client that stopped reading leaves its socket buffers full, so the result write blocks for as long as
the connection lasts, and so does a delivery already holding the mutex, with `close` waiting behind it.
A single such client kept the transport from closing at all.

The results are now written on a helper thread that `close` waits for at most `LISTEN_TEARDOWN_TIMEOUT`
(5 seconds) in all. When the time is up, every stream still open is closed from `close` itself: on a Ruby IO
that ends a write stalled on it with an `IOError` the helper rescues, while a stream object of another kind may
keep the helper blocked, which no longer holds `close`. Either way that stream goes without its result, like one
whose acknowledgement was never written, so the client re-sends. The helper is left to finish rather than
killed, so a write mutex is never dropped in the middle of a write. An error other than a lost peer raised by
a stream's write is reported through the exception reporter and no longer escapes `close`, which would
have skipped the remaining streams and the session cleanup.
Nor does an exception reporter that raises on that report: it is dropped, and the teardown goes on.
Whichever of the helper and `close` takes a stream out of the pending set closes it, so a stream is closed
once in either order.

## How Has This Been Tested?

Three new tests in `test/mcp/server/transports/streamable_http_transport_test.rb`: the teardown returns within
its timeout behind a held write mutex and behind a stalled result write, closing the stream and letting the helper
end on its own, and a result write failing with an unexpected error is reported while the other streams
still get their result. Against the previous library the third raises out of `close`, and a probe holding
a stream's write mutex while calling `close` never returns, where it returns after 5 seconds now.
A fourth test pairs the failing write with an exception reporter that raises: `close` still returns and tears
the rest down, where the previous library raised out of it.
The held-mutex and stalled-write tests count one close of their stream.

## Breaking Changes

None.
@koic
koic force-pushed the bound_the_listen_teardown_on_transport_close branch from eb8d3d2 to 03867b0 Compare October 11, 2026 16:27
@koic
koic merged commit 7735046 into modelcontextprotocol:main Oct 12, 2026
11 checks passed
@koic
koic deleted the bound_the_listen_teardown_on_transport_close branch October 12, 2026 16:29
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