mcp: add StreamableHTTPHandler.Close for graceful shutdown#1001
Open
piyushbag wants to merge 1 commit into
Open
mcp: add StreamableHTTPHandler.Close for graceful shutdown#1001piyushbag wants to merge 1 commit into
piyushbag wants to merge 1 commit into
Conversation
Expose public Close() to tear down all sessions and reject new requests with 503, matching the API proposed in modelcontextprotocol#440.
Author
|
Local verification on This matches the shutdown shape discussed in #440. Let me know if you want |
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.
Problem
StreamableHTTPHandlerholds stateful sessions in an internal map but had no public way to shut them down. Callers embedding the handler in an HTTP server (or running it in tests) could not reliably tear down all MCP sessions or block new ones during shutdown. The issue author proposed aClose()method; the codebase already had a privatecloseAll()helper with a TODO noting that a public API must also prevent new sessions from being added.Approach
Close() erroronStreamableHTTPHandler: set aclosedflag, snapshot and clear the sessions map under lock, then callsession.Close()on each entry outside the lock (avoids deadlocking with sessiononClosecallbacks).ServeHTTPwith503 Service Unavailableonce closed.Closeidempotent (second call is a no-op).closeAll()as a thin delegate toClose().TestStreamableHTTPHandlerClosecovering session teardown, empty map, idempotency, and 503 after close.Why this is better
Close() error, closes all sessions, prevents new sessions).defer handler.Close()inmainor test teardown without reaching into unexported fields.Fixes #440