Skip to content

Commit 0c77367

Browse files
author
OpenSourceMaintenance-Luccc-grok-4.5
committed
fix(server): terminate streamable HTTP sessions on manager shutdown
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" (#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 #3125 was closed unmerged during v2 backlog cleanup (maintainers pointed at #2253 on v1.x; this targets main with tests). Signed-off-by: Atlas/luccc-grok-4.5 <atlas@agent-studio.local>
1 parent a4f4ccd commit 0c77367

3 files changed

Lines changed: 129 additions & 2 deletions

File tree

src/mcp/server/streamable_http.py

Lines changed: 30 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -734,6 +734,8 @@ async def _handle_get_request(self, request: Request, send: Send) -> None:
734734

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

738740
async def standalone_sse_writer():
739741
try:
@@ -757,11 +759,13 @@ async def standalone_sse_writer():
757759
await sse_stream_writer.send(event_data)
758760
except anyio.ClosedResourceError:
759761
# Session teardown can close the stream while the writer is between dequeues.
762+
# Also expected when terminate()/close_sse_stream() closes the writer.
760763
pass
761764
except Exception:
762765
logger.exception("Error in standalone SSE writer") # pragma: no cover
763766
finally:
764767
logger.debug("Closing standalone SSE writer")
768+
self._sse_stream_writers.pop(GET_STREAM_KEY, None)
765769
await self._clean_up_memory_streams(GET_STREAM_KEY)
766770

767771
# Create and start EventSourceResponse
@@ -778,6 +782,7 @@ async def standalone_sse_writer():
778782
logger.exception("Error in standalone SSE response")
779783
await self._clean_up_memory_streams(GET_STREAM_KEY)
780784
finally:
785+
self._sse_stream_writers.pop(GET_STREAM_KEY, None)
781786
await sse_stream_writer.aclose()
782787
await sse_stream_reader.aclose()
783788

@@ -808,11 +813,19 @@ async def terminate(self) -> None:
808813
"""Terminate the current session, closing all streams.
809814
810815
Once terminated, all requests with this session ID will receive 404 Not Found.
816+
817+
Active SSE writers are closed first so EventSourceResponse / ASGI callables can
818+
complete instead of hanging until the task group is cancelled (see #2150).
811819
"""
812820

813821
self._terminated = True
814822
logger.info(f"Terminating session: {self.mcp_session_id}")
815823

824+
# Close SSE stream writers first so long-lived GET/POST SSE responses finish.
825+
# Copy keys: close_sse_stream mutates the dict (includes GET_STREAM_KEY).
826+
for request_id in list(self._sse_stream_writers.keys()):
827+
self.close_sse_stream(request_id)
828+
816829
# We need a copy of the keys to avoid modification during iteration
817830
request_stream_keys = list(self._request_streams.keys())
818831

@@ -910,8 +923,13 @@ async def _replay_events(self, last_event_id: str, request: Request, send: Send)
910923

911924
# Create SSE stream for replay
912925
sse_stream_writer, sse_stream_reader = anyio.create_memory_object_stream[SSEEvent](0)
926+
# Provisional key so terminate() can close the response during replay itself
927+
# (before stream_id is known from event_store.replay_events_after).
928+
replay_writer_key: RequestId = f"_replay:{last_event_id}:{id(sse_stream_writer)}"
929+
self._sse_stream_writers[replay_writer_key] = sse_stream_writer
913930

914931
async def replay_sender():
932+
registered_stream_id: RequestId | None = None
915933
try:
916934
async with sse_stream_writer:
917935
# Define an async callback for sending events
@@ -925,8 +943,10 @@ async def send_event(event_message: EventMessage) -> None:
925943
# If stream ID not in mapping, create it
926944
if stream_id and stream_id not in self._request_streams: # pragma: no branch
927945
try:
928-
# Register SSE writer so close_sse_stream() can close it
946+
# Re-key from provisional → stream_id for close_sse_stream(stream_id)
947+
self._sse_stream_writers.pop(replay_writer_key, None)
929948
self._sse_stream_writers[stream_id] = sse_stream_writer
949+
registered_stream_id = stream_id
930950

931951
# Prime the resumed connection so the client sees the stream
932952
# is re-registered. The replay→live-tail ordering window here
@@ -948,13 +968,20 @@ async def send_event(event_message: EventMessage) -> None:
948968

949969
await sse_stream_writer.send(event_data)
950970
finally:
951-
self._sse_stream_writers.pop(stream_id, None)
971+
# registered_stream_id is set immediately on try entry; keep the
972+
# guard for defensive cleanup if re-key is later reordered.
973+
if registered_stream_id is not None: # pragma: no branch
974+
self._sse_stream_writers.pop(registered_stream_id, None)
952975
await self._clean_up_memory_streams(stream_id)
953976
except anyio.ClosedResourceError: # pragma: lax no cover
954977
# Expected when close_sse_stream() is called
955978
logger.debug("Replay SSE stream closed by close_sse_stream()")
956979
except Exception: # pragma: lax no cover
957980
logger.exception("Error in replay sender")
981+
finally:
982+
self._sse_stream_writers.pop(replay_writer_key, None)
983+
if registered_stream_id is not None:
984+
self._sse_stream_writers.pop(registered_stream_id, None)
958985

959986
# Create and start EventSourceResponse
960987
response = EventSourceResponse(
@@ -968,6 +995,7 @@ async def send_event(event_message: EventMessage) -> None:
968995
except Exception: # pragma: lax no cover
969996
logger.exception("Error in replay response")
970997
finally:
998+
self._sse_stream_writers.pop(replay_writer_key, None)
971999
await sse_stream_writer.aclose()
9721000
await sse_stream_reader.aclose()
9731001

src/mcp/server/streamable_http_manager.py

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -153,6 +153,18 @@ async def lifespan(app: Starlette) -> AsyncIterator[None]:
153153
yield # Let the application run
154154
finally:
155155
logger.info("StreamableHTTP session manager shutting down")
156+
# Terminate active transports before cancelling the task group so
157+
# in-flight SSE responses can complete cleanly (issue #2150).
158+
active_transports = list(self._server_instances.values())
159+
for transport in active_transports:
160+
if not transport.is_terminated: # pragma: no branch
161+
try:
162+
await transport.terminate()
163+
except Exception: # pragma: no cover
164+
logger.exception(
165+
"Error terminating streamable HTTP session %s during shutdown",
166+
transport.mcp_session_id,
167+
)
156168
# Cancel task group to stop all spawned tasks
157169
tg.cancel_scope.cancel()
158170
self._task_group = None

tests/server/test_streamable_http_manager.py

Lines changed: 87 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -746,3 +746,90 @@ async def test_anonymous_session_accepts_anonymous_requests(
746746
session_id = await _open_session(manager, None)
747747

748748
assert await _request_session(manager, session_id, None) != 404
749+
750+
751+
@pytest.mark.anyio
752+
async def test_terminate_closes_active_sse_stream_writers():
753+
"""Regression for #2150: terminate must close SSE writers so ASGI can finish.
754+
755+
Without this, manager shutdown cancels the task group while EventSourceResponse
756+
is still open and uvicorn logs "ASGI callable returned without completing response".
757+
"""
758+
transport = StreamableHTTPServerTransport(mcp_session_id="test-session-2150")
759+
send_stream, receive_stream = anyio.create_memory_object_stream[object](1)
760+
transport._sse_stream_writers["req-1"] = send_stream # type: ignore[assignment]
761+
# Orphaned request stream with a key NOT also in _sse_stream_writers: close_sse_stream
762+
# only pops matching request streams, so terminate's remaining-keys loop must run.
763+
req_send, req_recv = anyio.create_memory_object_stream[object](1)
764+
transport._request_streams["req-orphan"] = (req_send, req_recv) # type: ignore[assignment]
765+
766+
await transport.terminate()
767+
768+
assert transport.is_terminated
769+
assert "req-1" not in transport._sse_stream_writers
770+
assert not transport._request_streams
771+
with pytest.raises(anyio.ClosedResourceError):
772+
await send_stream.send(object()) # type: ignore[arg-type]
773+
await receive_stream.aclose()
774+
775+
776+
@pytest.mark.anyio
777+
async def test_manager_shutdown_terminates_active_sessions():
778+
"""Regression for #2150: run() finally should terminate tracked transports."""
779+
app = Server("test-shutdown-terminate")
780+
manager = StreamableHTTPSessionManager(app=app)
781+
transport = StreamableHTTPServerTransport(mcp_session_id="shutdown-session")
782+
# Inject a live session as if a client still held an SSE connection.
783+
manager._server_instances[transport.mcp_session_id] = transport # type: ignore[index]
784+
original_terminate = transport.terminate
785+
terminate_calls = 0
786+
787+
async def counting_terminate() -> None:
788+
nonlocal terminate_calls
789+
terminate_calls += 1
790+
await original_terminate()
791+
792+
transport.terminate = counting_terminate # type: ignore[method-assign]
793+
794+
async with manager.run():
795+
assert transport.mcp_session_id in manager._server_instances
796+
# Exit context -> shutdown path should terminate then clear.
797+
798+
assert terminate_calls == 1
799+
assert transport.is_terminated
800+
assert transport.mcp_session_id not in manager._server_instances
801+
assert not manager._server_instances
802+
803+
804+
@pytest.mark.anyio
805+
async def test_terminate_closes_standalone_get_sse_writer_when_registered():
806+
"""GET standalone SSE writers must be registered so terminate can close them (#2150)."""
807+
from mcp.server.streamable_http import GET_STREAM_KEY
808+
809+
transport = StreamableHTTPServerTransport(mcp_session_id="get-session-2150")
810+
send_stream, receive_stream = anyio.create_memory_object_stream[object](1)
811+
transport._sse_stream_writers[GET_STREAM_KEY] = send_stream # type: ignore[assignment]
812+
813+
await transport.terminate()
814+
815+
assert transport.is_terminated
816+
assert GET_STREAM_KEY not in transport._sse_stream_writers
817+
with pytest.raises(anyio.ClosedResourceError):
818+
await send_stream.send(object()) # type: ignore[arg-type]
819+
await receive_stream.aclose()
820+
821+
822+
@pytest.mark.anyio
823+
async def test_terminate_closes_provisional_replay_sse_writer():
824+
"""Replay writers registered under provisional keys must close on terminate."""
825+
transport = StreamableHTTPServerTransport(mcp_session_id="replay-session-2150")
826+
send_stream, receive_stream = anyio.create_memory_object_stream[object](1)
827+
key = "_replay:evt-1:123"
828+
transport._sse_stream_writers[key] = send_stream # type: ignore[assignment]
829+
830+
await transport.terminate()
831+
832+
assert key not in transport._sse_stream_writers
833+
with pytest.raises(anyio.ClosedResourceError):
834+
await send_stream.send(object()) # type: ignore[arg-type]
835+
await receive_stream.aclose()

0 commit comments

Comments
 (0)