Skip to content

ClientSessionGroup: a rejected connect_to_server leaves its transport running — the session is established before its components are validated #3490

Description

@Kushal9889

Initial Checks

Release line

2.x (current stable), reproduced at 9972c21a on mcp 2.2.0.

Description

ClientSessionGroup.connect_to_server opens the transport first and validates the server's components second. When the duplicate-name check rejects the server, the transport it already opened is never closed, and the caller is never handed the session, so nothing else can close it either.

The order in session_group.py is:

  1. _establish_session launches the transport, runs initialize, stores the stack in self._session_exit_stacks[session], and enters it into self._exit_stack.
  2. _aggregate_components lists the components and hits raise MCPError(..., message=f"{matching_tools} already exist in group tools.").
  3. self._sessions[session] = component_names is on the line after that raise, so it never runs.

That leaves the connection live but unreachable:

  • group.sessions reads self._sessions, which the rejected server never reached, so it is not listed.
  • disconnect_from_server(session) would close it, but it needs the ClientSession object and connect_to_server raised instead of returning one.
  • self._exit_stack still holds the stack, so it is released only when the whole group tears down.

For stdio that is a live child process. For streamable HTTP it is an initialized session on a server that counts sessions against max_sessions. Both grow by one per rejection.

This is the case the docs treat as ordinary rather than exotic. docs/client/session-groups.md says two servers you don't control "will collide eventually", and its !!! check fence tells the reader to run exactly this and see the MCPError. The same page says the error is "raised before anything from the second server is registered" — true of the three component dicts, but not of the connection that was opened to read them.

It costs a long-lived host that connects servers dynamically, or retries a failed connect: one more process or session per attempt, none of them reclaimable until the group closes.

Expected: a connect_to_server that raises should close the transport it opened before the exception leaves, so "nothing from the second server is registered" covers its connection too. If holding it is deliberate, the session needs to be reachable — attached to the raised MCPError, or listed by group.sessions — so the caller can close it.

Two notes to save review time:

#3228 looks like the same shape one layer down — a request the server refuses still leaves a registered session behind because the session is created before validation.

Example Code

import asyncio
import os
import subprocess
import sys
import tempfile
from pathlib import Path

from mcp import ClientSessionGroup, MCPError, StdioServerParameters

SERVER = """
from mcp.server import MCPServer

mcp = MCPServer({name!r})


@mcp.tool()
def search(query: str) -> str:
    \"\"\"Search.\"\"\"
    return f"{name} got {{query!r}}"


if __name__ == "__main__":
    mcp.run()
"""


def children() -> set[int]:
    out = subprocess.run(["pgrep", "-P", str(os.getpid())], capture_output=True, text=True).stdout.split()
    return {int(p) for p in out}


async def main() -> None:
    tmp = Path(tempfile.mkdtemp())
    for name in ("Library", "Web"):
        (tmp / f"{name}.py").write_text(SERVER.format(name=name))

    py = sys.executable
    library = StdioServerParameters(command=py, args=[str(tmp / "Library.py")])
    web = StdioServerParameters(command=py, args=[str(tmp / "Web.py")])

    async with ClientSessionGroup() as group:
        await group.connect_to_server(library)
        base = children()

        for attempt in range(1, 4):
            try:
                await group.connect_to_server(web)
            except MCPError as err:
                print(f"rejection #{attempt}: {err}")
            print(
                f"   live subprocesses left behind : {len(children() - base)}\n"
                f"   group.sessions                : {len(group.sessions)}\n"
                f"   group.tools                   : {sorted(group.tools)}"
            )


if __name__ == "__main__":
    asyncio.run(main())

Output on 2.2.0:

rejection #1: {'search'} already exist in group tools.
   live subprocesses left behind : 1
   group.sessions                : 1
   group.tools                   : ['search']
rejection #2: {'search'} already exist in group tools.
   live subprocesses left behind : 2
   group.sessions                : 1
   group.tools                   : ['search']
rejection #3: {'search'} already exist in group tools.
   live subprocesses left behind : 3
   group.sessions                : 1
   group.tools                   : ['search']

Same growth on StreamableHttpParameters against two HTTP servers, where what accumulates is an initialized session rather than a process — len(group._session_exit_stacks) - len(group._sessions) goes 1, 2, 3 while group.sessions stays at 1.

origin/v1.x has the same ordering — stack stored at session_group.py:348, entered into _exit_stack at :351, the duplicate check raises at :433, and self._sessions[session] is set at :438 — so 1.x looks affected too, though I only ran the reproduction on 2.2.0.

Python & MCP Python SDK

Python 3.10.20, mcp 2.2.0, starlette via httpx2, macOS 26.5.2 (arm64)

I used AI assistance to narrow this down and to build the reproduction; I ran it myself and can walk through the code path. If you'd like an outside PR for it, I'd like to take the fix.

Activity

  1. added
    v2Affects the v2 line (2.x on main)
    v1Affects the v1.x maintenance line
    on Sep 10, 2026
  2. Kushal9889 commented on Sep 10, 2026

    @Kushal9889
    Author

    Opened #3491 with the fix and a regression test: the transport is closed in connect_to_server, the only caller that owns one, so connect_with_session keeps its "the group never closes a session it didn't open" contract.

    The new test fails on main (assert closed) and passes with the change. Locally: 5968 tests pass, coverage stays at 100.00% with no new pragma: no cover, and ruff/pyright/uv lock --check are clean.

    I'm not assigned here, so the intake gate will close it until you decide — that's fine, it's on the table rather than jumping the queue, and I'll push any changes as new commits so it can reopen cleanly.

  3. Yatsuiii commented on Sep 17, 2026

    @Yatsuiii

    I'd like to pick this up if someone can assign me.

    #3491 and #3502 both proposed the same fix and were auto-closed by the linked-issue bot rather than reviewed, so nobody currently holds the assignment.

    I reproduced the leak independently against 9972c21. After connect_to_server() raises on a tool name collision, the rejected server's stdio child is still running, because the session's exit stack was already registered with the group's long-lived stack before _aggregate_components() raised. Ran it 10 times unpatched (child survives 10/10) and 10 times with #3502's patch applied (clean 10/10).

    If assigned, I'd send the fix with a regression test that checks the child process is actually gone, rather than asserting on a mocked exit stack. The existing test in #3502 mocks AsyncExitStack, so it passes whether or not the real transport dies.

  4. esSJay7 commented on Oct 2, 2026

    @esSJay7

    Hi — I independently reproduced this on current main and have a local fix with regression coverage that verifies the rejected connection is actually cleaned up before the exception escapes. I also checked the earlier closed PRs (#3491, #3502, #3523) and have not opened another PR because I see there are already contributors interested in the issue.

    If maintainers would like another outside contribution here, I'm happy to take it and submit the cleaned patch/tests. Please assign #3490 to me if so; otherwise I’ll leave the existing queue alone.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    bugSomething isn't workingv1Affects the v1.x maintenance linev2Affects the v2 line (2.x on main)

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions