Skip to content

Commit 1b50a12

Browse files
committed
End Streamable HTTP sessions through one path
Review follow-ups. The session manager now discards a session -- forgets it and terminates its transport, shielded from cancellation -- through one helper, used when the client deletes the session, when the session task ends, and when the request that opens the session is refused, fails or is cancelled; that bracket now starts when the session is registered, so a session whose task cannot be started is discarded as well. A stateless transport is terminated even when its request is cancelled, so the per-request task always ends. A transport whose idle period has run out answers as terminated instead of dispatching into a message loop that is shutting down. Idle timeouts must be finite as well as positive. Tests assert that discarded sessions' transports are terminated, cover the two new cleanup paths and the expired-transport answer, and the idle-hold tests check the suspended countdown directly and arm the short timeout only once the holding request is in flight instead of racing a timer.
1 parent ae2daca commit 1b50a12

4 files changed

Lines changed: 253 additions & 113 deletions

File tree

src/mcp/server/streamable_http.py

Lines changed: 12 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -199,12 +199,12 @@ def __init__(
199199
200200
Raises:
201201
ValueError: If the session ID contains invalid characters, or if `idle_timeout`
202-
is not a positive number.
202+
is not a positive, finite number.
203203
"""
204204
if mcp_session_id is not None and not SESSION_ID_PATTERN.fullmatch(mcp_session_id):
205205
raise ValueError("Session ID must only contain visible ASCII characters (0x21-0x7E)")
206-
if idle_timeout is not None and idle_timeout <= 0:
207-
raise ValueError("idle_timeout must be a positive number of seconds")
206+
if idle_timeout is not None and not (math.isfinite(idle_timeout) and idle_timeout > 0):
207+
raise ValueError("idle_timeout must be a positive, finite number of seconds")
208208

209209
self.mcp_session_id = mcp_session_id
210210
self.is_json_response_enabled = is_json_response_enabled
@@ -477,6 +477,15 @@ async def handle_request(self, scope: Scope, receive: Receive, send: Send) -> No
477477
await self._handle_request(scope, receive, send)
478478
return
479479

480+
if self.idle_scope.cancel_called:
481+
# The idle period already ran out and the host is ending this
482+
# session: answer as terminated rather than dispatch into a
483+
# message loop that is going away.
484+
if not self._terminated:
485+
await self.terminate()
486+
await self._handle_request(scope, receive, send)
487+
return
488+
480489
# A request in flight (an open GET stream included) holds the session:
481490
# the idle countdown is suspended while any is being served and
482491
# restarts when the last one completes.

src/mcp/server/streamable_http_manager.py

Lines changed: 38 additions & 41 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44

55
import contextlib
66
import logging
7+
import math
78
from collections.abc import AsyncIterator
89
from typing import TYPE_CHECKING, Any, Final
910
from uuid import uuid4
@@ -94,8 +95,8 @@ def __init__(
9495
max_request_body_size: int = DEFAULT_MAX_REQUEST_BODY_SIZE,
9596
max_sessions: int | None = DEFAULT_MAX_SESSIONS,
9697
):
97-
if session_idle_timeout is not None and session_idle_timeout <= 0:
98-
raise ValueError("session_idle_timeout must be a positive number of seconds")
98+
if session_idle_timeout is not None and not (math.isfinite(session_idle_timeout) and session_idle_timeout > 0):
99+
raise ValueError("session_idle_timeout must be a positive, finite number of seconds")
99100
if max_request_body_size <= 0:
100101
raise ValueError("max_request_body_size must be a positive number of bytes")
101102
if max_sessions is not None and max_sessions <= 0:
@@ -247,16 +248,15 @@ async def run_stateless_server(*, task_status: TaskStatus[None] = anyio.TASK_STA
247248
except Exception: # pragma: lax no cover
248249
logger.exception("Stateless session crashed")
249250

250-
# Assert task group is not None for type checking
251+
# The per-request server task only ends once the transport is
252+
# terminated, so terminate it even if the request was cancelled.
251253
assert self._task_group is not None
252-
# Start the server task
253-
await self._task_group.start(run_stateless_server)
254-
255-
# Handle the HTTP request and return the response
256-
await http_transport.handle_request(scope, receive, send)
257-
258-
# Terminate the transport after the request is handled
259-
await http_transport.terminate()
254+
try:
255+
await self._task_group.start(run_stateless_server)
256+
await http_transport.handle_request(scope, receive, send)
257+
finally:
258+
with anyio.CancelScope(shield=True):
259+
await http_transport.terminate()
260260

261261
async def _handle_stateful_request(self, scope: Scope, receive: Receive, send: Send) -> None:
262262
"""Process request in stateful mode - maintaining session state between requests."""
@@ -283,7 +283,7 @@ async def _handle_stateful_request(self, scope: Scope, receive: Receive, send: S
283283
if transport.is_terminated:
284284
# The client ended the session (DELETE): forget it now rather
285285
# than when its server task winds down.
286-
self._forget_session(request_mcp_session_id)
286+
await self._discard_session(request_mcp_session_id, transport)
287287
return
288288

289289
if request_mcp_session_id is None:
@@ -305,13 +305,6 @@ async def _handle_stateful_request(self, scope: Scope, receive: Receive, send: S
305305
idle_timeout=self.session_idle_timeout,
306306
)
307307

308-
assert http_transport.mcp_session_id is not None
309-
if requestor is not None:
310-
self._session_owners[http_transport.mcp_session_id] = requestor
311-
self._server_instances[http_transport.mcp_session_id] = http_transport
312-
logger.info(f"Created new transport with session ID: {new_session_id}")
313-
314-
# Define the server runner
315308
async def run_server(*, task_status: TaskStatus[None] = anyio.TASK_STATUS_IGNORED) -> None:
316309
async with http_transport.connect() as streams:
317310
read_stream, write_stream = streams
@@ -342,43 +335,47 @@ async def run_server(*, task_status: TaskStatus[None] = anyio.TASK_STATUS_IGNORE
342335
logger.exception(f"Session {new_session_id} crashed")
343336
finally:
344337
# However the session ended (client DELETE, idle
345-
# timeout, crash), stop tracking it and make sure the
346-
# transport refuses anything that still reaches it.
347-
self._forget_session(new_session_id)
348-
if not http_transport.is_terminated:
349-
await http_transport.terminate()
350-
351-
# Assert task group is not None for type checking
352-
assert self._task_group is not None
353-
# Start the server task
354-
await self._task_group.start(run_server)
355-
356-
# Handle the HTTP request and return the response. Without a
357-
# session ID only an initialize request can succeed, so if this
358-
# one was refused nothing was established: forget the session
359-
# again rather than keep it (and its server task) around.
338+
# timeout, crash), discard it.
339+
await self._discard_session(new_session_id, http_transport)
340+
341+
if requestor is not None:
342+
self._session_owners[new_session_id] = requestor
343+
self._server_instances[new_session_id] = http_transport
344+
logger.info(f"Created new transport with session ID: {new_session_id}")
345+
346+
# Without a session ID only an initialize request can succeed,
347+
# so if this one is refused, fails or is cancelled (or the
348+
# session's server task cannot even be started) nothing was
349+
# established: discard the session again rather than keep it
350+
# (and its server task) around.
360351
established = False
361352
try:
353+
assert self._task_group is not None
354+
await self._task_group.start(run_server)
362355
status = await _send_and_report_status(http_transport.handle_request, scope, receive, send)
363356
established = status is not None and status < 400
364357
finally:
365358
if not established: # pragma: no branch
366-
# Refused, failed or cancelled before a session was
367-
# established: nothing to keep.
368-
self._forget_session(new_session_id)
369-
with anyio.CancelScope(shield=True):
370-
await http_transport.terminate()
359+
await self._discard_session(new_session_id, http_transport)
371360
else:
372361
# Unknown or expired session ID - return 404 per MCP spec
373362
# TODO(L62): Align error code once spec clarifies
374363
# See: https://github.com/modelcontextprotocol/python-sdk/issues/1821
375364
logger.info(f"Rejected request with unknown or expired session ID: {request_mcp_session_id[:64]}")
376365
await _error_response("Session not found", 404)(scope, receive, send)
377366

378-
def _forget_session(self, session_id: str) -> None:
379-
"""Stop tracking a session; requests naming it are answered 404 from then on."""
367+
async def _discard_session(self, session_id: str, transport: StreamableHTTPServerTransport) -> None:
368+
"""Stop tracking the session and make sure its transport refuses anything that still reaches it.
369+
370+
The session is forgotten first, before any await, so its ID answers 404
371+
from the moment this is called; terminating the transport is shielded so
372+
it completes even while the caller is being cancelled.
373+
"""
380374
self._server_instances.pop(session_id, None)
381375
self._session_owners.pop(session_id, None)
376+
if not transport.is_terminated:
377+
with anyio.CancelScope(shield=True):
378+
await transport.terminate()
382379

383380

384381
def _error_response(message: str, status_code: int) -> Response:

0 commit comments

Comments
 (0)