fix: prevent stack overflow in transport close with re-entrancy guard (#1788)
Co-authored-by: Felix Weinberger <fweinberger@anthropic.com> Co-authored-by: Felix Weinberger <3823880+felixweinberger@users.noreply.github.com>
This commit is contained in:
parent
653c5d000c
commit
df4b6cc88d
3 changed files with 48 additions and 0 deletions
5
.changeset/fix-streamable-close-reentrant.md
Normal file
5
.changeset/fix-streamable-close-reentrant.md
Normal file
|
|
@ -0,0 +1,5 @@
|
|||
---
|
||||
'@modelcontextprotocol/server': patch
|
||||
---
|
||||
|
||||
Prevent stack overflow in StreamableHTTPServerTransport.close() with re-entrant guard
|
||||
|
|
@ -225,6 +225,7 @@ export class WebStandardStreamableHTTPServerTransport implements Transport {
|
|||
// when sessionId is not set (undefined), it means the transport is in stateless mode
|
||||
private sessionIdGenerator: (() => string) | undefined;
|
||||
private _started: boolean = false;
|
||||
private _closed: boolean = false;
|
||||
private _streamMapping: Map<string, StreamMapping> = new Map();
|
||||
private _requestToStreamMapping: Map<RequestId, string> = new Map();
|
||||
private _requestResponseMap: Map<RequestId, JSONRPCMessage> = new Map();
|
||||
|
|
@ -897,6 +898,11 @@ export class WebStandardStreamableHTTPServerTransport implements Transport {
|
|||
}
|
||||
|
||||
async close(): Promise<void> {
|
||||
if (this._closed) {
|
||||
return;
|
||||
}
|
||||
this._closed = true;
|
||||
|
||||
// Close all SSE connections
|
||||
for (const { cleanup } of this._streamMapping.values()) {
|
||||
cleanup();
|
||||
|
|
|
|||
|
|
@ -956,4 +956,41 @@ describe('Zod v4', () => {
|
|||
expect(error?.message).toContain('Unsupported protocol version');
|
||||
});
|
||||
});
|
||||
|
||||
describe('close() re-entrancy guard', () => {
|
||||
it('should not recurse when onclose triggers a second close()', async () => {
|
||||
const transport = new WebStandardStreamableHTTPServerTransport({ sessionIdGenerator: randomUUID });
|
||||
|
||||
let closeCallCount = 0;
|
||||
transport.onclose = () => {
|
||||
closeCallCount++;
|
||||
// Simulate the Protocol layer calling close() again from within onclose —
|
||||
// the re-entrancy guard should prevent infinite recursion / stack overflow.
|
||||
void transport.close();
|
||||
};
|
||||
|
||||
// Should resolve without throwing RangeError: Maximum call stack size exceeded
|
||||
await expect(transport.close()).resolves.toBeUndefined();
|
||||
expect(closeCallCount).toBe(1);
|
||||
});
|
||||
|
||||
it('should clean up all streams exactly once even when close() is called concurrently', async () => {
|
||||
const transport = new WebStandardStreamableHTTPServerTransport({ sessionIdGenerator: randomUUID });
|
||||
|
||||
const cleanupCalls: string[] = [];
|
||||
|
||||
// Inject a fake stream entry to verify cleanup runs exactly once
|
||||
// @ts-expect-error accessing private map for test purposes
|
||||
transport._streamMapping.set('stream-1', {
|
||||
cleanup: () => {
|
||||
cleanupCalls.push('stream-1');
|
||||
}
|
||||
});
|
||||
|
||||
// Fire two concurrent close() calls — only the first should proceed
|
||||
await Promise.all([transport.close(), transport.close()]);
|
||||
|
||||
expect(cleanupCalls).toEqual(['stream-1']);
|
||||
});
|
||||
});
|
||||
});
|
||||
|
|
|
|||
Loading…
Reference in a new issue