Skip to content

Cleanly terminated streamable-HTTP sessions are never deregistered: the DELETE path skips its own cleanup #3300

Description

@sainikhiljuluri

Credit

This was found and reported by @pete-builds in a comment on #3228, which ended with an offer to split it out. I own #3228, so I am taking them up on that. This is their finding, not an independent discovery.

What this issue adds is isolation. The repro in that comment establishes each session with a bare GET (no Accept: text/event-stream), which is itself refused with 406 — so every session it counts was created by a rejected request, i.e. by #3228. That evidence cannot separate "leaked because refused" from "leaked because DELETEd." The repro below establishes sessions with a real initialize handshake returning 200, leaving the DELETE as the only variable.

This is orthogonal to #3229. That PR discards a session only when the establishing request returns ≥ 400; a successful initialize returns 200, so its branch never fires here. The run below is on the #3229 branch and still leaks.

Description

On the handshake-era streamable-HTTP path, a session terminated cleanly by DELETE is never removed from StreamableHTTPSessionManager._server_instances.

StreamableHTTPServerTransport.terminate() sets self._terminated = True before closing the streams that let the session task unwind. When run_server's finally block runs, its and not http_transport.is_terminated conjunct is false, so the del is skipped — for exactly the sessions that shut down correctly. The only other removal is _server_instances.clear() at manager shutdown.

The sharpest form of this is not "a dict grows." It is that the guard defeats the SDK's only mitigation, and penalizes well-behaved clients: with session_idle_timeout configured, a session whose client simply vanishes is reclaimed; a session the client explicitly DELETEs is not. Terminating a session politely is strictly worse than abandoning it rudely.

Scope, stated up front

  • Handshake-era path only (2024-11-052025-11-25). _handle_request routes any MCP-Protocol-Version outside HANDSHAKE_PROTOCOL_VERSIONS to handle_modern_request, and _streamable_http_modern.py has zero references to _server_instances. The 2.x client default mode="auto" negotiates 2026-07-28 and leaks nothing (scenario D below).
  • Sessions and DELETE carry removed_in="2026-07-28" in this repo's own conformance table (SEP-2567). This is a bug on a deprecated-but-still-supported path — I would rather say that myself than have you say it.
  • It still covers the entire pre-2026 installed base and any mode="legacy" client, where terminate_on_close=True is the default, so the DELETE is the happy path there.
  • stateless_http=True is immune. session_idle_timeout is not a workaround (scenario B) and is not a parameter of streamable_http_app() or MCPServer anyway.

Reproduction

Real uvicorn on a real socket, this SDK's own client, no monkeypatching and no in-process ASGI shim.

A leaked : sessions=3 mode=legacy delete=True  idle_timeout=None | registry immediate=3 after_checkpoints=3 after_2.0s=3 is_terminated=[True, True, True]
B leaked : sessions=3 mode=legacy delete=True  idle_timeout=0.3  | registry immediate=3 after_checkpoints=3 after_3.0s=3 is_terminated=[True, True, True]
C reaped : sessions=3 mode=legacy delete=False idle_timeout=0.3  | registry immediate=3 after_checkpoints=3 after_3.0s=0 is_terminated=[]
D modern : sessions=3 mode=auto   delete=True  idle_timeout=None | registry immediate=0 after_checkpoints=0 after_1.0s=0 is_terminated=[]

B vs C is the whole bug. Same idle timeout; the client that politely DELETEs leaks, the client that vanishes is reaped. The registry is drained through 1000 event-loop checkpoints plus a settle window before each count, so this is not a premature read.

repro.py
"""Repro: streamable-HTTP session registry leak on cleanly terminated sessions."""

from __future__ import annotations

import contextlib
import socket

import anyio
import uvicorn
from starlette.applications import Starlette
from starlette.routing import Mount

from mcp.client.client import Client
from mcp.client.streamable_http import streamable_http_client
from mcp.server.lowlevel.server import Server
from mcp.server.streamable_http_manager import StreamableHTTPSessionManager
from mcp.server.transport_security import TransportSecuritySettings


def free_port() -> int:
    s = socket.socket()
    s.bind(("127.0.0.1", 0))
    port = s.getsockname()[1]
    s.close()
    return port


@contextlib.asynccontextmanager
async def serve(manager: StreamableHTTPSessionManager):
    @contextlib.asynccontextmanager
    async def lifespan(_app):
        async with manager.run():
            yield

    star = Starlette(routes=[Mount("/mcp", app=manager.handle_request)], lifespan=lifespan)
    port = free_port()
    server = uvicorn.Server(uvicorn.Config(star, host="127.0.0.1", port=port, log_level="error"))
    async with anyio.create_task_group() as tg:
        tg.start_soon(server.serve)
        for _ in range(500):
            if server.started:
                break
            await anyio.sleep(0.02)
        try:
            yield f"http://127.0.0.1:{port}/mcp"
        finally:
            server.should_exit = True
            await anyio.sleep(0.3)
            tg.cancel_scope.cancel()


async def scenario(
    name: str, *, n: int, mode: str, terminate_on_close: bool, idle_timeout: float | None, settle: float
) -> None:
    manager = StreamableHTTPSessionManager(
        app=Server("repro"),
        session_idle_timeout=idle_timeout,
        security_settings=TransportSecuritySettings(enable_dns_rebinding_protection=False),
    )
    async with serve(manager) as url:
        for _ in range(n):
            async with Client(streamable_http_client(url, terminate_on_close=terminate_on_close), mode=mode):
                pass
        immediate = len(manager._server_instances)
        for _ in range(1000):  # drain every pending event-loop checkpoint
            await anyio.sleep(0)
        checkpoints = len(manager._server_instances)
        await anyio.sleep(settle)
        settled = len(manager._server_instances)
        terminated = [t.is_terminated for t in manager._server_instances.values()]
        print(
            f"{name}: sessions={n} mode={mode} delete={terminate_on_close} idle_timeout={idle_timeout} "
            f"| registry immediate={immediate} after_checkpoints={checkpoints} "
            f"after_{settle}s={settled} is_terminated={terminated}"
        )


async def main() -> None:
    await scenario("A leaked ", n=3, mode="legacy", terminate_on_close=True, idle_timeout=None, settle=2.0)
    await scenario("B leaked ", n=3, mode="legacy", terminate_on_close=True, idle_timeout=0.3, settle=3.0)
    await scenario("C reaped ", n=3, mode="legacy", terminate_on_close=False, idle_timeout=0.3, settle=3.0)
    await scenario("D modern ", n=3, mode="auto", terminate_on_close=True, idle_timeout=None, settle=1.0)


anyio.run(main, backend="asyncio")

The retained transport is inert — terminate() does empty _request_streams and _sse_stream_writers, and the orphan does not reference the Server app, the manager, the task group, tool arguments, or tool results. It is a dead stub of a few KB per session, never reclaimed, growing linearly. I am deliberately not quoting a precise byte figure: my two measurement harnesses disagreed by several KB because same-process client allocations contaminate both.

_session_owners leaks identically — its pop sits inside the same if — but only when auth is in play, since it is written only for an AuthenticatedUser. It stores an authorization_context(user); I checked that the raw bearer token is not reachable from it, so this is not credential retention.

Root cause

git log -S "not http_transport.is_terminated" returns exactly one commit: 7b1078b5 "Fix: Prevent session manager shutdown on individual session crash (#841)". The whole try/except/finally arrived there for crash cleanup, with the guard under the comment # Only remove from instances if not terminated.

It is not a race guard. The idle reaper did not exist yet, and every terminate path that exists today pops before calling terminate(), so the in self._server_instances check short-circuits first. What the guard actually buys is a nicer 404 body: retention routes post-DELETE requests to the transport's own _terminated check ("Not Found: Session has been terminated") instead of the manager's "Session not found".

Suggested fix

Pop in the same turn that answers the request, rather than leaving it to the session task's finally:

# streamable_http_manager.py, existing-session branch, after handle_request:
            await transport.handle_request(scope, receive, send)
+           if transport.is_terminated:
+               self._server_instances.pop(request_mcp_session_id, None)
+               self._session_owners.pop(request_mcp_session_id, None)
            return

plus dropping and not http_transport.is_terminated from the finally guard.

Deregistering synchronously matters. Dropping the guard alone also fixes the leak, but then deregistration happens asynchronously a few event-loop checkpoints later, leaving a window where a follow-up request could observe either 404 body — a flaky assertion waiting to happen. With the patch above, immediate=0 in every scenario.

SseServerTransport is the clean counterexample: it pops _session_owners unconditionally in its finally, with no is_terminated-style guard. The streamable-HTTP manager is the outlier.

The objection you are going to raise

tests/interaction/transports/test_hosting_session.py currently asserts session_id in manager._server_instances, with a comment framing the retention as deliberate — "the manager keeps the terminated transport registered, so the next request reaches the transport's own _terminated check."

This fix does change observable behavior, and I would rather flag that than have it found in review. The post-termination 404 body changes from "Not Found: Session has been terminated" to "Session not found", and that 404 stops echoing Mcp-Session-Id. That assertion and its snapshot would need updating.

Why I think it is still right:

  • The status code (404) and JSON-RPC error code (-32600) are unchanged, and the conformance requirement hosting:session:post-termination-404 reads "answered with 404 Not Found" — status only, no body. The spec's MUST is on the status code.
  • The special message was never reliable. Idle-reaped, crashed, and gracefully-exited sessions have always answered "Session not found"; only DELETE got the nicer string. Three of four termination modes already behave the way this fix makes the fourth behave.
  • Session ids are uuid4().hex, so a forgotten id can never be revived — the manager's unknown-session branch answers 404 for it indefinitely.
  • A deliberate tombstone would be a bounded set of ids with eviction, not a permanent strong reference to a whole transport in two dicts with no reclamation path.

Prior art

I am happy to open a PR with the fix and the test updates if you would like it.

Environment

Reproduced on upstream/main and on the #3229 branch. Also present in released mcp 1.29.0 per @pete-builds' original report.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions