fix: prevent session + task leak on GET/DELETE without session-id

Closed
RA
12d ago
+225-1

Summary

In stateful mode, StreamableHTTPSessionManager currently allocates a
StreamableHTTPServerTransport and spawns a background task for every
request without an MCP-Session-Id header — including GET and DELETE,
which the MCP spec does not allow to initialize a session. The transport
rejects the request downstream (400/406), but the allocated session and
task are never cleaned up. Only session_idle_timeout (opt-in, off by
default) reaps them later.

This PR moves the "must have a session-id" check up into the manager for
GET and DELETE, so those requests are rejected with 400 Bad Request: Missing session ID before any session is allocated.

Repro (before the patch)

Fire 100 GET /mcp requests with an SDK-only in-process server (no
FastMCP, no external transport), then inspect _server_instances:

[SNAPSHOT baseline                           ] sessions=   0  tasks=   0
[SNAPSHOT after 100x GET /mcp/ (no header)   ] sessions= 100  tasks= 100
[SNAPSHOT after 100x GET /mcp/ (Accept hdr)  ] sessions= 200  tasks= 200
[SNAPSHOT after 100x DELETE /mcp/            ] sessions= 300  tasks= 300

Every one of those 300 sessions is still in _server_instances, and
every one has a corresponding suspended anyio task in the task group. In
production this shows up as Created new transport with session ID: …
in the server logs with no matching Session terminated / closed / ended / deleted events.

We caught this via a Docker healthcheck polling GET /mcp every 30s on
a taylorwilsdon/google_workspace_mcp container (which uses this SDK
via FastMCP) running for 10 days: 28 800 sessions accumulated, RAM
climbed from ~100 MiB to ~1.3 GiB (~46 KiB / leaked session), 0
restarts, container reported healthy the whole time.

Repro after the patch

Same script, same requests:

[SNAPSHOT after 100x GET /mcp/ (no header)   ] sessions=   0  tasks=   0
[SNAPSHOT after 100x GET /mcp/ (Accept hdr)  ] sessions=   0  tasks=   0
[SNAPSHOT after 100x DELETE /mcp/            ] sessions=   0  tasks=   0

Requests are rejected at the manager layer with a well-formed JSON-RPC
error:

{
  "jsonrpc": "2.0",
  "id": "server-error",
  "error": {
    "code": -32600,
    "message": "Bad Request: Missing session ID"
  }
}

Why the manager and not the transport

The transport-level rejection happens after the manager has already
allocated a session and spawned the background task. That task waits on
serve_loop / app.run(read_stream, write_stream, …) which never
returns because read_stream never receives anything, so the finally
cleanup at the bottom of the task never fires. Moving the check up-stack
means we never enter the code path that leaks.

What the change touches

  • src/mcp/server/streamable_http_manager.py:
    • pre-instantiates a TransportSecurityMiddleware on the manager so
      the DNS-rebinding / bad-Host check runs before session allocation
      (previously that check lived only in the transport, so a bad-Host
      request would first allocate a session, then get rejected — same
      leak),
    • rejects GET and DELETE without a session-id with
      400 "Missing session ID" (same message the transport uses for the
      equivalent POST case at streamable_http.py:844, kept identical to
      avoid diverging error surfaces),
    • leaves PUT/PATCH/OPTIONS alone — they continue to reach the
      transport's _handle_unsupported_request and get the existing
      405 Method Not Allowed response.
  • tests/server/test_streamable_http_manager.py:
    • adds a parametrized regression test
      test_non_post_without_session_id_does_not_allocate_session[GET, DELETE]
      that asserts _server_instances and the task group both stay empty
      after a bad request, and that the response is the JSON-RPC 400
      shape.

Full suite passes locally on v1.x:

  • tests/server/test_streamable_http_manager.py — all pass (existing +
    2 new).
  • tests/server/test_streamable_http_security.py — passes
    (test_streamable_http_security_get_request still returns 421 for
    bad Host, verifying the reorder didn't break DNS-rebinding).
  • tests/shared/test_streamable_http.py — passes
    (test_method_not_allowed still returns 405 for PUT, verifying the
    fix only intercepts GET/DELETE).
  • tests/issues/test_1363_race_condition_streamable_http.py — passes.

Total: 82 passed, 0 failed on v1.x.

Behavioural notes for downstream users

  • POST /mcp without a session-id: unchanged — still initializes a
    new session.
  • GET /mcp without a session-id: was 406 Not Acceptable (or 400 if
    the client sent the right Accept header); now consistently
    400 "Bad Request: Missing session ID".
  • DELETE /mcp without a session-id: was 400 "Missing session ID"
    from the transport (with a leaked session); now
    400 "Missing session ID" from the manager (no leak).
  • PUT/PATCH/OPTIONS without a session-id: unchanged — still
    405 Method Not Allowed.

The response body shape is the same JSON-RPC error the manager already
uses for unknown-session cases, so clients that already parse that
shape need no changes.