Skip to content

fix(server): terminate streamable HTTP sessions on manager shutdown - #3218

Closed
baiyuxi930826 wants to merge 1 commit into
modelcontextprotocol:mainfrom
baiyuxi930826:atlas/fix-2150-shutdown-sessions-main
Closed

fix(server): terminate streamable HTTP sessions on manager shutdown#3218
baiyuxi930826 wants to merge 1 commit into
modelcontextprotocol:mainfrom
baiyuxi930826:atlas/fix-2150-shutdown-sessions-main

Conversation

@baiyuxi930826

Copy link
Copy Markdown

Summary

Fixes #2150 on current main.

When shutting down a Streamable HTTP server (CTRL+C / lifespan exit) with live SSE clients, uvicorn logs:

ERROR: ASGI callable returned without completing response.

Root causes (still present on main @ a4f4ccd)

  1. StreamableHTTPSessionManager.run() finally cancelled the task group and cleared _server_instances without await transport.terminate() on active sessions.
  2. StreamableHTTPServerTransport.terminate() closed request/read/write streams but left _sse_stream_writers open, so EventSourceResponse never completed.

Fix

  • Manager shutdown: terminate each non-terminated transport before cancelling the task group.
  • terminate(): close all SSE writers first via close_sse_stream (covers POST request streams, GET standalone, and provisional replay keys).
  • Register the GET standalone SSE writer immediately; use a provisional writer key during event replay so mid-replay shutdown can still close the ASGI response.

Relation to other work

Verification

uv run pytest tests/server/test_streamable_http_manager.py::test_terminate_closes_active_sse_stream_writers \
  tests/server/test_streamable_http_manager.py::test_manager_shutdown_terminates_active_sessions \
  tests/server/test_streamable_http_manager.py::test_terminate_closes_standalone_get_sse_writer_when_registered \
  tests/server/test_streamable_http_manager.py::test_terminate_closes_provisional_replay_sse_writer -q
# 4 passed

uv run pytest tests/server/test_streamable_http_manager.py -q
# 34 passed

Test plan

  • New unit regressions for terminate + manager shutdown
  • Full test_streamable_http_manager.py suite green locally (34)
  • CI on this PR

Why: On CTRL+C / lifespan exit with live SSE clients, StreamableHTTPSessionManager
cancelled the task group without terminating transports, and terminate() left
_sse_stream_writers open — uvicorn then logs "ASGI callable returned without
completing response" (modelcontextprotocol#2150).

- Manager run() finally: await terminate() on active transports before cancel
- terminate(): close all SSE writers first (incl. GET + provisional replay keys)
- Register GET standalone writer immediately; provisional key during replay
- Regression tests for writer close + manager shutdown path

Rebased onto current main after modelcontextprotocol#3125 was closed unmerged during v2 backlog
cleanup (maintainers pointed at modelcontextprotocol#2253 on v1.x; this targets main with tests).

Signed-off-by: Atlas/luccc-grok-4.5 <atlas@agent-studio.local>

@cubic-dev-ai cubic-dev-ai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

2 issues found across 3 files

Prompt for AI agents (unresolved issues)

Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="src/mcp/server/streamable_http_manager.py">

<violation number="1" location="src/mcp/server/streamable_http_manager.py:158">
P1: A session created while this loop awaits an earlier `terminate()` is omitted from the snapshot and reaches task-group cancellation with its SSE response still open. Serialize creation against shutdown (or set/check a shutdown flag before creating transports) so every accepted session is terminated before cancellation.</violation>
</file>

<file name="src/mcp/server/streamable_http.py">

<violation number="1" location="src/mcp/server/streamable_http.py:738">
P1: Concurrent standalone GETs can still both pass the one-stream check because it only observes `_request_streams`, which is populated later by the writer task. The second request overwrites this newly registered writer, and either response's cleanup can then remove the other registration; that leaves a live SSE response that `terminate()` cannot close. Consider making the duplicate-stream guard use the writer registration (or reserving the GET key atomically) and only removing a mapping when it still refers to that handler's writer.</violation>
</file>

Reply with feedback, questions, or to request a fix.

Re-trigger cubic

logger.info("StreamableHTTP session manager shutting down")
# Terminate active transports before cancelling the task group so
# in-flight SSE responses can complete cleanly (issue #2150).
active_transports = list(self._server_instances.values())

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1: A session created while this loop awaits an earlier terminate() is omitted from the snapshot and reaches task-group cancellation with its SSE response still open. Serialize creation against shutdown (or set/check a shutdown flag before creating transports) so every accepted session is terminated before cancellation.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At src/mcp/server/streamable_http_manager.py, line 158:

<comment>A session created while this loop awaits an earlier `terminate()` is omitted from the snapshot and reaches task-group cancellation with its SSE response still open. Serialize creation against shutdown (or set/check a shutdown flag before creating transports) so every accepted session is terminated before cancellation.</comment>

<file context>
@@ -153,6 +153,18 @@ async def lifespan(app: Starlette) -> AsyncIterator[None]:
                 logger.info("StreamableHTTP session manager shutting down")
+                # Terminate active transports before cancelling the task group so
+                # in-flight SSE responses can complete cleanly (issue #2150).
+                active_transports = list(self._server_instances.values())
+                for transport in active_transports:
+                    if not transport.is_terminated:  # pragma: no branch
</file context>

# Create SSE stream
sse_stream_writer, sse_stream_reader = anyio.create_memory_object_stream[SSEEvent](0)
# Register immediately so session terminate() can close this ASGI response.
self._sse_stream_writers[GET_STREAM_KEY] = sse_stream_writer

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1: Concurrent standalone GETs can still both pass the one-stream check because it only observes _request_streams, which is populated later by the writer task. The second request overwrites this newly registered writer, and either response's cleanup can then remove the other registration; that leaves a live SSE response that terminate() cannot close. Consider making the duplicate-stream guard use the writer registration (or reserving the GET key atomically) and only removing a mapping when it still refers to that handler's writer.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At src/mcp/server/streamable_http.py, line 738:

<comment>Concurrent standalone GETs can still both pass the one-stream check because it only observes `_request_streams`, which is populated later by the writer task. The second request overwrites this newly registered writer, and either response's cleanup can then remove the other registration; that leaves a live SSE response that `terminate()` cannot close. Consider making the duplicate-stream guard use the writer registration (or reserving the GET key atomically) and only removing a mapping when it still refers to that handler's writer.</comment>

<file context>
@@ -734,6 +734,8 @@ async def _handle_get_request(self, request: Request, send: Send) -> None:
         # Create SSE stream
         sse_stream_writer, sse_stream_reader = anyio.create_memory_object_stream[SSEEvent](0)
+        # Register immediately so session terminate() can close this ASGI response.
+        self._sse_stream_writers[GET_STREAM_KEY] = sse_stream_writer
 
         async def standalone_sse_writer():
</file context>

@github-actions github-actions Bot added the missing-issue-link Auto-closed: PR needs a linked issue assigned to its author (see CONTRIBUTING.md) label Aug 17, 2026
@github-actions

Copy link
Copy Markdown
Contributor

Thanks for the contribution. This repository only keeps pull requests open when they're linked to an issue that a maintainer has assigned to the author — CONTRIBUTING.md explains why and how we work. This PR has been closed for now because you aren't currently assigned to #2150.

If a maintainer would like this change as a PR from you, they'll assign you to #2150 and this PR will reopen automatically — there's nothing more you need to do. (If you opened the issue, this PR already shows up on its timeline.)

There's no need to open a new PR — this one will be reopened. While it's closed, please push any updates as new commits rather than force-pushing, since GitHub can't reopen a PR whose branch has been rewritten.

Maintainers: reopening this PR, removing the missing-issue-link label, or adding bypass-issue-check bypasses the check.

@github-actions github-actions Bot closed this Aug 17, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

missing-issue-link Auto-closed: PR needs a linked issue assigned to its author (see CONTRIBUTING.md)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Active Streamable HTTP sessions are not terminated during shutdown

1 participant