Repository navigation
Conversation
koic
force-pushed
the
bound_the_listen_teardown_on_transport_close
branch
2 times, most recently
from
October 11, 2026 16:26
9944ae6 to
eb8d3d2
Compare
## 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
force-pushed
the
bound_the_listen_teardown_on_transport_close
branch
from
October 11, 2026 16:27
eb8d3d2 to
03867b0
Compare
atesgoral
approved these changes
Oct 11, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Motivation and Context
StreamableHTTPTransport#closewrites each opensubscriptions/listenstream itsSubscriptionsListenResultbefore 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, withclosewaiting behind it. A single such client kept the transport from closing at all.The results are now written on a helper thread that
closewaits for at mostLISTEN_TEARDOWN_TIMEOUT(5 seconds) in all. When the time is up, every stream still open is closed fromcloseitself: on a Ruby IO that ends a write stalled on it with anIOErrorthe helper rescues, while a stream object of another kind may keep the helper blocked, which no longer holdsclose. 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 escapesclose, 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 andclosetakes 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 ofclose, and a probe holding a stream's write mutex while callingclosenever returns, where it returns after 5 seconds now. A fourth test pairs the failing write with an exception reporter that raises:closestill 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
Checklist