Skip to content

Commit cbdbced

Browse files
committed
Forget Streamable HTTP sessions when they end
The session manager kept a session's registry entry after the client ended it with DELETE (the per-session task's cleanup skipped terminated transports), and a request without a session ID that was refused (anything but a valid initialize: wrong Accept, malformed JSON, a non-initialize message, GET/DELETE) still left a registered transport with a running server task behind it. Now the manager drops the entry as soon as the transport is terminated, the per-session task forgets the session and terminates its transport however the loop ended, and a provisional session whose opening request was answered with an error is discarded before the request returns. A follow-up request on a deleted session is answered by the manager ("Session not found", 404) rather than by the dead transport.
1 parent 6705402 commit cbdbced

5 files changed

Lines changed: 247 additions & 115 deletions

File tree

src/mcp/server/streamable_http_manager.py

Lines changed: 47 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@
1414
from mcp_types.version import HANDSHAKE_PROTOCOL_VERSIONS
1515
from starlette.requests import Request
1616
from starlette.responses import Response
17-
from starlette.types import Receive, Scope, Send
17+
from starlette.types import ASGIApp, Message, Receive, Scope, Send
1818

1919
from mcp.server._streamable_http_modern import handle_modern_request
2020
from mcp.server.auth.middleware.bearer_auth import AuthenticatedUser, AuthorizationContext, authorization_context
@@ -278,6 +278,10 @@ async def _handle_stateful_request(self, scope: Scope, receive: Receive, send: S
278278
if transport.idle_scope is not None and self.session_idle_timeout is not None:
279279
transport.idle_scope.deadline = anyio.current_time() + self.session_idle_timeout # pragma: no cover
280280
await transport.handle_request(scope, receive, send)
281+
if transport.is_terminated:
282+
# The client ended the session (DELETE): forget it now rather
283+
# than when its server task winds down.
284+
self._forget_session(request_mcp_session_id)
281285
return
282286

283287
if request_mcp_session_id is None:
@@ -327,33 +331,37 @@ async def run_server(*, task_status: TaskStatus[None] = anyio.TASK_STATUS_IGNORE
327331
)
328332

329333
if idle_scope.cancelled_caught:
330-
assert http_transport.mcp_session_id is not None
331-
logger.info(f"Session {http_transport.mcp_session_id} idle timeout")
332-
self._server_instances.pop(http_transport.mcp_session_id, None)
333-
self._session_owners.pop(http_transport.mcp_session_id, None)
334-
await http_transport.terminate()
334+
logger.info(f"Session {new_session_id} idle timeout")
335335
except Exception:
336-
logger.exception(f"Session {http_transport.mcp_session_id} crashed")
336+
logger.exception(f"Session {new_session_id} crashed")
337337
finally:
338-
if ( # pragma: no branch
339-
http_transport.mcp_session_id
340-
and http_transport.mcp_session_id in self._server_instances
341-
and not http_transport.is_terminated
342-
):
343-
logger.info(
344-
"Cleaning up crashed session "
345-
f"{http_transport.mcp_session_id} from active instances."
346-
)
347-
del self._server_instances[http_transport.mcp_session_id]
348-
self._session_owners.pop(http_transport.mcp_session_id, None)
338+
# However the session ended (client DELETE, idle
339+
# timeout, crash), stop tracking it and make sure the
340+
# transport refuses anything that still reaches it.
341+
self._forget_session(new_session_id)
342+
if not http_transport.is_terminated:
343+
await http_transport.terminate()
349344

350345
# Assert task group is not None for type checking
351346
assert self._task_group is not None
352347
# Start the server task
353348
await self._task_group.start(run_server)
354349

355-
# Handle the HTTP request and return the response
356-
await http_transport.handle_request(scope, receive, send)
350+
# Handle the HTTP request and return the response. Without a
351+
# session ID only an initialize request can succeed, so if this
352+
# one was refused nothing was established: forget the session
353+
# again rather than keep it (and its server task) around.
354+
established = False
355+
try:
356+
status = await _send_and_report_status(http_transport.handle_request, scope, receive, send)
357+
established = status is not None and status < 400
358+
finally:
359+
if not established:
360+
# Refused, failed or cancelled before a session was
361+
# established: nothing to keep.
362+
self._forget_session(new_session_id)
363+
with anyio.CancelScope(shield=True):
364+
await http_transport.terminate()
357365
else:
358366
# Unknown or expired session ID - return 404 per MCP spec
359367
# TODO(L62): Align error code once spec clarifies
@@ -367,6 +375,25 @@ async def run_server(*, task_status: TaskStatus[None] = anyio.TASK_STATUS_IGNORE
367375
)
368376
await response(scope, receive, send)
369377

378+
def _forget_session(self, session_id: str) -> None:
379+
"""Stop tracking a session; requests naming it are answered 404 from then on."""
380+
self._server_instances.pop(session_id, None)
381+
self._session_owners.pop(session_id, None)
382+
383+
384+
async def _send_and_report_status(app: ASGIApp, scope: Scope, receive: Receive, send: Send) -> int | None:
385+
"""Run `app` for one request and return the HTTP status it answered with (None if it sent no response)."""
386+
status: int | None = None
387+
388+
async def watch_status(message: Message) -> None:
389+
nonlocal status
390+
if message["type"] == "http.response.start":
391+
status = message["status"]
392+
await send(message)
393+
394+
await app(scope, receive, watch_status)
395+
return status
396+
370397

371398
class StreamableHTTPASGIApp:
372399
"""ASGI application for Streamable HTTP server transport."""

tests/interaction/transports/test_hosting_session.py

Lines changed: 3 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -107,23 +107,15 @@ async def test_delete_terminates_the_session_and_subsequent_requests_return_404(
107107
delete = await http.delete("/mcp", headers=base_headers(session_id=session_id))
108108
assert delete.status_code == 200
109109

110-
# The manager keeps the terminated transport registered, so the next request reaches the
111-
# transport's own _terminated check rather than the manager's unknown-session path.
112-
assert session_id in manager._server_instances
110+
# The manager forgets a terminated session, so from then on the ID is simply unknown.
111+
assert session_id not in manager._server_instances
113112
post = await http.post(
114113
"/mcp",
115114
json={"jsonrpc": "2.0", "id": 2, "method": "tools/list"},
116115
headers=base_headers(session_id=session_id),
117116
)
118117
assert (post.status_code, post.json()) == snapshot(
119-
(
120-
404,
121-
{
122-
"jsonrpc": "2.0",
123-
"id": None,
124-
"error": {"code": -32600, "message": "Not Found: Session has been terminated"},
125-
},
126-
)
118+
(404, {"jsonrpc": "2.0", "id": None, "error": {"code": -32600, "message": "Session not found"}})
127119
)
128120

129121

0 commit comments

Comments
 (0)