From dac3d4ed4a033573f4c2e658fd49b6bd4f4d68a3 Mon Sep 17 00:00:00 2001 From: kmaclip Date: Fri, 27 Mar 2026 11:31:07 -0400 Subject: [PATCH 1/2] fix: prevent stack overflow in transport close with re-entrancy guard When 15-25+ transports close simultaneously, close() -> onclose() triggers the Protocol layer to call close() again recursively, causing a stack overflow. The process stays alive but becomes unresponsive. Three problems in the original close(): 1. No re-entrancy guard: onclose() can call back into close() 2. Iterating _streamMapping while cleanup callbacks delete from it 3. No error isolation: one failing cleanup prevents others The fix: - Adds _closing boolean guard to prevent recursive calls - Snapshots stream values into an array and clears the map before iterating, avoiding mutation-during-iteration - Wraps each cleanup() in try/catch so one failure doesn't block remaining stream cleanups - Calls onclose() only after all cleanup is complete Note: two prior PRs (#1700, #1705) attempted similar fixes but were closed. This implementation combines the best elements of both: the re-entrancy guard from #1700 and the snapshot-before-clear pattern from #1705, with the addition of error isolation. Closes #1699 --- packages/server/src/server/streamableHttp.ts | 29 ++++++++++++++++---- 1 file changed, 24 insertions(+), 5 deletions(-) diff --git a/packages/server/src/server/streamableHttp.ts b/packages/server/src/server/streamableHttp.ts index 31053f35c..2c06d5968 100644 --- a/packages/server/src/server/streamableHttp.ts +++ b/packages/server/src/server/streamableHttp.ts @@ -899,15 +899,34 @@ export class WebStandardStreamableHTTPServerTransport implements Transport { return undefined; } + private _closing = false; + async close(): Promise { - // Close all SSE connections - for (const { cleanup } of this._streamMapping.values()) { - cleanup(); + // Guard against re-entrant calls. When onclose() triggers the + // Protocol layer to call close() again, this prevents infinite + // recursion that causes a stack overflow with many transports. + if (this._closing) { + return; } - this._streamMapping.clear(); + this._closing = true; - // Clear any pending responses + // Snapshot and clear before iterating to avoid issues with + // cleanup callbacks that modify the map during iteration. + const streams = Array.from(this._streamMapping.values()); + this._streamMapping.clear(); this._requestResponseMap.clear(); + + // Close all SSE connections with error isolation so one + // failing cleanup doesn't prevent others from running. + for (const { cleanup } of streams) { + try { + cleanup(); + } catch { + // Individual stream cleanup failures should not + // prevent other streams from being cleaned up. + } + } + this.onclose?.(); } From ff06d699ee4d3ef1656ff3c86756a8098a124a4a Mon Sep 17 00:00:00 2001 From: kmaclip Date: Fri, 27 Mar 2026 15:06:07 -0400 Subject: [PATCH 2/2] fix: use spread operator instead of Array.from to satisfy lint rule Replace Array.from(map.values()) with [...map.values()] to fix the unicorn/prefer-spread ESLint rule. --- packages/server/src/server/streamableHttp.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/server/src/server/streamableHttp.ts b/packages/server/src/server/streamableHttp.ts index 2c06d5968..f7d12598d 100644 --- a/packages/server/src/server/streamableHttp.ts +++ b/packages/server/src/server/streamableHttp.ts @@ -912,7 +912,7 @@ export class WebStandardStreamableHTTPServerTransport implements Transport { // Snapshot and clear before iterating to avoid issues with // cleanup callbacks that modify the map during iteration. - const streams = Array.from(this._streamMapping.values()); + const streams = [...this._streamMapping.values()]; this._streamMapping.clear(); this._requestResponseMap.clear();