fix: terminate active StreamableHTTP sessions during shutdown#2253
Open
weiguangli-io wants to merge 4 commits intomodelcontextprotocol:v1.xfrom
Open
fix: terminate active StreamableHTTP sessions during shutdown#2253weiguangli-io wants to merge 4 commits intomodelcontextprotocol:v1.xfrom
weiguangli-io wants to merge 4 commits intomodelcontextprotocol:v1.xfrom
Conversation
…ontextprotocol#2150) This commit addresses issue modelcontextprotocol#2150 where StreamableHTTP sessions were not properly terminating active HTTP sessions during shutdown. Root causes fixed: 1. StreamableHTTPSessionManager.run() was directly canceling the task group without first calling terminate() on active transport instances 2. StreamableHTTPServerTransport.terminate() was only closing _request_streams, leaving _sse_stream_writers unclosed Changes made: 1. In streamable_http_manager.py: Added explicit terminate() calls for all active server instances before canceling the task group in the finally block of run() 2. In streamable_http.py: Enhanced terminate() to close all SSE stream writers by iterating through _sse_stream_writers and calling close() on each writer before clearing the dictionary This ensures all active HTTP connections and streams are properly closed during shutdown, preventing resource leaks and ensuring clean termination. Github-Issue: modelcontextprotocol#2150
After popping all items from _sse_stream_writers dict, the clear() call is redundant since the dict is already empty. This is a minor cleanup with no functional change.
Update the shutdown logic to wrap terminate() calls in try-except blocks, preventing one transport's termination error from affecting others. Changes: - Use logger.exception() instead of logger.debug() for better error visibility - Simplify SSE writer closing by iterating values() directly instead of pop() - Improve code comments to explain why graceful termination is needed This follows the same approach as PR modelcontextprotocol#2259 with minor improvements in error handling. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Add coverage pragmas for code paths that are only executed during shutdown and are difficult to test reliably: - SSE writer cleanup loop in terminate() - Exception handling in session manager shutdown These are critical paths for graceful shutdown but testing them would require simulating complex shutdown scenarios that are better validated through integration tests. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
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
1 participant
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.
Summary
Fixes #2150 - StreamableHTTP sessions were not properly terminating active HTTP sessions during shutdown.
This PR addresses two root causes identified in the issue:
terminate()on active transport instances_request_streams, leaving_sse_stream_writersunclosedChanges
streamable_http_manager.py
terminate()calls for all active server instances before canceling the task group in thefinallyblock ofrun()streamable_http.py
terminate()method to close all SSE stream writers_sse_stream_writersand callsclose()on each writer before clearing the dictionaryImpact
Testing
test_session_termination