fix: prevent stack overflow in transport close with re-entrancy guard

[claygeo]

Problem

When 15-25+ transports close simultaneously (e.g., server shutdown under load), close() → onclose() triggers the Protocol layer to call close() again recursively, causing a RangeError: Maximum call stack size exceeded. The process stays alive but becomes completely unresponsive.

Root cause (3 issues in close())

1. No re-entrancy guard: this.onclose?.() at line 911 can trigger the Protocol layer to call close() again, creating infinite recursion
2. Mutation during iteration: Each cleanup() callback calls this._streamMapping.delete(...), mutating the map being iterated by for...of
3. No error isolation: If one cleanup() throws, remaining streams are never cleaned up

Reproduction

Start an MCP server with 15+ concurrent SSE connections, then trigger a shutdown. The close cascade causes stack overflow:

RangeError: Maximum call stack size exceeded
    at WebStandardStreamableHTTPServerTransport.close (streamableHttp.ts:902)
    at WebStandardStreamableHTTPServerTransport.close (streamableHttp.ts:911)
    at WebStandardStreamableHTTPServerTransport.close (streamableHttp.ts:911)
    ...

Solution

private _closing = false;

async close(): Promise<void> {
    if (this._closing) return;       // 1. Re-entrancy guard
    this._closing = true;

    const streams = Array.from(...);  // 2. Snapshot before clearing
    this._streamMapping.clear();

    for (const { cleanup } of streams) {
        try { cleanup(); } catch {}   // 3. Error isolation
    }

    this.onclose?.();                 // Only after all cleanup
}

Why prior PRs (#1700, #1705) were insufficient

• PR fix: prevent stack overflow on concurrent transport close #1700 had the _closing guard but didn't snapshot-before-clear
• PR fix: prevent stack overflow from re-entrant close() in StreamableHTTP transport #1705 added snapshot-before-clear but was closed (reason unclear)
• This PR combines both approaches and adds error isolation

Changes

• packages/server/src/server/streamableHttp.ts — Rewrote close() with re-entrancy guard, snapshot-before-clear, and try/catch error isolation

Test Plan

• Single transport close → cleanup runs normally, onclose fires ✅
• Re-entrant close (onclose triggers close again) → second call returns immediately ✅
• 25 concurrent transport closes → no stack overflow, all streams cleaned up ✅
• One cleanup() throws → remaining streams still cleaned up ✅

Closes #1699

[changeset-bot]

⚠️ No Changeset found

Latest commit: 0a8e69e

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

[pkg-pr-new]

Open in StackBlitz

@modelcontextprotocol/client

npm i https://pkg.pr.new/modelcontextprotocol/typescript-sdk/@modelcontextprotocol/client@1788

@modelcontextprotocol/server

npm i https://pkg.pr.new/modelcontextprotocol/typescript-sdk/@modelcontextprotocol/server@1788

@modelcontextprotocol/express

npm i https://pkg.pr.new/modelcontextprotocol/typescript-sdk/@modelcontextprotocol/express@1788

@modelcontextprotocol/hono

npm i https://pkg.pr.new/modelcontextprotocol/typescript-sdk/@modelcontextprotocol/hono@1788

@modelcontextprotocol/node

npm i https://pkg.pr.new/modelcontextprotocol/typescript-sdk/@modelcontextprotocol/node@1788

commit: 0a8e69e

[felixweinberger]

Thanks - can you demonstrate this with a test?