Repository navigation
Conversation
## Motivation and Context `StreamableHTTPTransport#close` stopped the session reaper with `Thread#kill`. The reaper removes expired sessions under the transport's lock and closes their streams after releasing it, so a kill landing between the two left those streams open with no session left to find them by: nothing closed them afterwards, since `close` tears down only the sessions still registered. The reaper now waits between reaps on a condition variable under the lock instead of sleeping, and `close` sets a flag, signals it, and joins the thread for up to `REAPER_JOIN_TIMEOUT` seconds before killing it. A reaper waiting leaves its wait at once; one past its removal finishes closing the streams it collected before `close` goes on to the rest of the transport. The join is bounded so a stream whose close never returns cannot hold the shutdown, and its outcome never decides whether the teardown runs: `Thread#join` re-raises the exception a thread died with, which `close` swallows, since the reaper reported what it could when it happened and the sessions still have to go. A spurious wakeup only runs a reap early. ## How Has This Been Tested? Three new tests in `test/mcp/server/transports/streamable_http_transport_test.rb`: one wakes the reaper while a reaped session's stream blocks inside its close, and checks that `close` returns only once that close completed and that the stream was closed; one ends the reaper with an exception, through a reap that raises and an exception reporter that raises as well, and checks that `close` still tears the sessions and their streams down; one makes the join time out at once while the reaper is inside a close that never returns, and checks that `close` returns and kills it. Against the previous library, with the reaper's sleep stubbed out so that it reaps at once, `close` returned while the stream was still open and the stream was never closed; the second and the third are what a plain join would have failed, with `close` re-raising the reaper's exception before any teardown, and never returning. ## Breaking Changes None. `close` now waits up to five seconds for a reap in progress to finish closing its streams.
atesgoral
approved these changes
Oct 9, 2026
This branch has not been deployed
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#closestopped the session reaper withThread#kill. The reaper removes expired sessions under the transport's lock and closes their streams after releasing it, so a kill landing between the two left those streams open with no session left to find them by: nothing closed them afterwards, sinceclosetears down only the sessions still registered.The reaper now waits between reaps on a condition variable under the lock instead of sleeping, and
closesets a flag, signals it, and joins the thread for up toREAPER_JOIN_TIMEOUTseconds before killing it. A reaper waiting leaves its wait at once; one past its removal finishes closing the streams it collected beforeclosegoes on to the rest of the transport. The join is bounded so a stream whose close never returns cannot hold the shutdown, and its outcome never decides whether the teardown runs:Thread#joinre-raises the exception a thread died with, whichcloseswallows, since the reaper reported what it could when it happened and the sessions still have to go. A spurious wakeup only runs a reap early.How Has This Been Tested?
Three new tests in
test/mcp/server/transports/streamable_http_transport_test.rb: one wakes the reaper while a reaped session's stream blocks inside its close, and checks thatclosereturns only once that close completed and that the stream was closed; one ends the reaper with an exception, through a reap that raises and an exception reporter that raises as well, and checks thatclosestill tears the sessions and their streams down; one makes the join time out at once while the reaper is inside a close that never returns, and checks thatclosereturns and kills it. Against the previous library, with the reaper's sleep stubbed out so that it reaps at once,closereturned while the stream was still open and the stream was never closed; the second and the third are what a plain join would have failed, withclosere-raising the reaper's exception before any teardown, and never returning.Breaking Changes
None.
closenow waits up to five seconds for a reap in progress to finish closing its streams.Types of changes
Checklist