Skip to content

fix: async onclose, stdin EOF detection, SIGTERM in examples #1814

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
MayCXC wants to merge 1 commit into
base: main
Choose a base branch
from
Open

fix: async onclose, stdin EOF detection, SIGTERM in examples #1814

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 11 additions & 0 deletions .changeset/async-onclose-stdin-eof.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
---
"@modelcontextprotocol/core": patch
"@modelcontextprotocol/server": patch
"@modelcontextprotocol/client": patch
---

Allow async `onclose` callbacks on Transport and Protocol. The signature changes from `() => void` to `() => void | Promise<void>`, and all call sites now await the callback. This lets MCP servers perform async cleanup (e.g., releasing browser sessions or database connections) when the transport closes.

Close `StdioServerTransport` when stdin reaches EOF, so containerized servers exit cleanly on client disconnect.

Add SIGTERM handlers alongside SIGINT in all examples, since MCP servers run as background processes stopped by SIGTERM, not interactively via Ctrl+C.
4 changes: 4 additions & 0 deletions examples/client/src/elicitationUrlExample.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -813,6 +813,10 @@
console.log('\nReceived SIGINT. Cleaning up...');
await cleanup();
});
process.on('SIGTERM', async () => {
console.log('\nReceived SIGINT. Cleaning up...');
await cleanup();
});

Check warning on line 819 in examples/client/src/elicitationUrlExample.ts

View check run for this annotation

Claude / Claude Code Review

SIGTERM handlers log wrong signal name 

The SIGTERM handler in `elicitationUrlExample.ts` (line 817) and `simpleStreamableHttp.ts` logs `'\nReceived SIGINT. Cleaning up...'` instead of `'\nReceived SIGTERM. Cleaning up...'`. This copy-paste error means operators monitoring logs will see misleading signal names when the process is stopped via SIGTERM (e.g., by container runtimes or process managers).
Comment on lines +816 to +819
Copy link
Copy Markdown

@claude claude bot Apr 9, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 The SIGTERM handler in elicitationUrlExample.ts (line 817) and simpleStreamableHttp.ts logs '\nReceived SIGINT. Cleaning up...' instead of '\nReceived SIGTERM. Cleaning up...'. This copy-paste error means operators monitoring logs will see misleading signal names when the process is stopped via SIGTERM (e.g., by container runtimes or process managers).

Extended reasoning...

What the bug is: In both examples/client/src/elicitationUrlExample.ts and examples/client/src/simpleStreamableHttp.ts, the newly-added SIGTERM handler was copy-pasted from the SIGINT handler but the log string was not updated. It still reads 'Received SIGINT. Cleaning up...' inside process.on('SIGTERM', ...).


// Start the interactive client
try {
Expand Down
5 changes: 5 additions & 0 deletions examples/client/src/simpleOAuthClient.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -448,6 +448,11 @@ async function main(): Promise<void> {
client.close();
process.exit(0);
});
process.on('SIGTERM', () => {
console.log('\n\n👋 Goodbye!');
client.close();
process.exit(0);
});

try {
await client.connect();
Expand Down
4 changes: 4 additions & 0 deletions examples/client/src/simpleStreamableHttp.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -997,6 +997,10 @@ process.on('SIGINT', async () => {
console.log('\nReceived SIGINT. Cleaning up...');
await cleanup();
});
process.on('SIGTERM', async () => {
console.log('\nReceived SIGINT. Cleaning up...');
await cleanup();
});

// Start the interactive client
try {
Expand Down
16 changes: 16 additions & 0 deletions examples/server/src/elicitationFormExample.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -453,6 +453,22 @@ async function main() {
process.on('SIGINT', async () => {
console.log('Shutting down server...');

// Close all active transports to properly clean up resources
for (const sessionId in transports) {
try {
console.log(`Closing transport for session ${sessionId}`);
await transports[sessionId]!.close();
delete transports[sessionId];
} catch (error) {
console.error(`Error closing transport for session ${sessionId}:`, error);
}
}
console.log('Server shutdown complete');
process.exit(0);
});
process.on('SIGTERM', async () => {
console.log('Shutting down server...');

// Close all active transports to properly clean up resources
for (const sessionId in transports) {
try {
Expand Down
17 changes: 17 additions & 0 deletions examples/server/src/elicitationUrlExample.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -731,3 +731,20 @@ process.on('SIGINT', async () => {
console.log('Server shutdown complete');
process.exit(0);
});
process.on('SIGTERM', async () => {
console.log('Shutting down server...');

// Close all active transports to properly clean up resources
for (const sessionId in transports) {
try {
console.log(`Closing transport for session ${sessionId}`);
await transports[sessionId]!.close();
delete transports[sessionId];
delete sessionsNeedingElicitation[sessionId];
} catch (error) {
console.error(`Error closing transport for session ${sessionId}:`, error);
}
}
console.log('Server shutdown complete');
process.exit(0);
});
4 changes: 4 additions & 0 deletions examples/server/src/jsonResponseStreamableHttp.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -163,3 +163,7 @@ process.on('SIGINT', async () => {
console.log('Shutting down server...');
process.exit(0);
});
process.on('SIGTERM', async () => {
console.log('Shutting down server...');
process.exit(0);
});
5 changes: 5 additions & 0 deletions examples/server/src/simpleStatelessStreamableHttp.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -169,3 +169,8 @@ process.on('SIGINT', async () => {
// eslint-disable-next-line unicorn/no-process-exit
process.exit(0);
});
process.on('SIGTERM', async () => {
console.log('Shutting down server...');
// eslint-disable-next-line unicorn/no-process-exit
process.exit(0);
});
16 changes: 16 additions & 0 deletions examples/server/src/simpleStreamableHttp.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -815,3 +815,19 @@ process.on('SIGINT', async () => {
console.log('Server shutdown complete');
process.exit(0);
});
process.on('SIGTERM', async () => {
console.log('Shutting down server...');

// Close all active transports to properly clean up resources
for (const sessionId in transports) {
try {
console.log(`Closing transport for session ${sessionId}`);
await transports[sessionId]!.close();
delete transports[sessionId];
} catch (error) {
console.error(`Error closing transport for session ${sessionId}:`, error);
}
}
console.log('Server shutdown complete');
process.exit(0);
});
15 changes: 15 additions & 0 deletions examples/server/src/simpleTaskInteractive.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -741,3 +741,18 @@ process.on('SIGINT', async () => {
console.log('Server shutdown complete');
process.exit(0);
});
process.on('SIGTERM', async () => {
console.log('\nShutting down server...');
for (const sessionId of Object.keys(transports)) {
try {
await transports[sessionId]!.close();
delete transports[sessionId];
} catch (error) {
console.error(`Error closing session ${sessionId}:`, error);
}
}
taskStore.cleanup();
messageQueue.cleanup();
console.log('Server shutdown complete');
process.exit(0);
});
18 changes: 18 additions & 0 deletions examples/server/src/standaloneSseWithGetStreamableHttp.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -159,3 +159,21 @@ process.on('SIGINT', async () => {
console.log('Server shutdown complete');
process.exit(0);
});
process.on('SIGTERM', async () => {
console.log('Shutting down server...');
clearInterval(resourceChangeInterval);

// Close all active transports to properly clean up resources
for (const sessionId in transports) {
try {
console.log(`Closing transport for session ${sessionId}`);
await transports[sessionId]!.close();
delete transports[sessionId];
delete servers[sessionId];
} catch (error) {
console.error(`Error closing transport for session ${sessionId}:`, error);
}
}
console.log('Server shutdown complete');
process.exit(0);
});
4 changes: 2 additions & 2 deletions packages/client/src/client/sse.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,7 @@ export class SSEClientTransport implements Transport {
private _fetchWithInit: FetchLike;
private _protocolVersion?: string;

onclose?: () => void;
onclose?: () => void | Promise<void>;
onerror?: (error: Error) => void;
onmessage?: (message: JSONRPCMessage) => void;

Expand Down Expand Up @@ -242,7 +242,7 @@ export class SSEClientTransport implements Transport {
async close(): Promise<void> {
this._abortController?.abort();
this._eventSource?.close();
this.onclose?.();
await this.onclose?.();
}

async send(message: JSONRPCMessage): Promise<void> {
Expand Down
6 changes: 3 additions & 3 deletions packages/client/src/client/stdio.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -96,7 +96,7 @@ export class StdioClientTransport implements Transport {
private _serverParams: StdioServerParameters;
private _stderrStream: PassThrough | null = null;

onclose?: () => void;
onclose?: () => void | Promise<void>;
onerror?: (error: Error) => void;
onmessage?: (message: JSONRPCMessage) => void;

Expand Down Expand Up @@ -139,9 +139,9 @@ export class StdioClientTransport implements Transport {
resolve();
});

this._process.on('close', _code => {
this._process.on('close', async _code => {
this._process = undefined;
this.onclose?.();
await this.onclose?.();
});

this._process.stdin?.on('error', error => {
Expand Down
4 changes: 2 additions & 2 deletions packages/client/src/client/streamableHttp.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -152,7 +152,7 @@ export class StreamableHTTPClientTransport implements Transport {
private _serverRetryMs?: number; // Server-provided retry delay from SSE retry field
private _reconnectionTimeout?: ReturnType<typeof setTimeout>;

onclose?: () => void;
onclose?: () => void | Promise<void>;
onerror?: (error: Error) => void;
onmessage?: (message: JSONRPCMessage) => void;

Expand Down Expand Up @@ -463,7 +463,7 @@ export class StreamableHTTPClientTransport implements Transport {
this._reconnectionTimeout = undefined;
}
this._abortController?.abort();
this.onclose?.();
await this.onclose?.();
}

async send(
Expand Down
6 changes: 3 additions & 3 deletions packages/client/src/client/websocket.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ export class WebSocketClientTransport implements Transport {
private _socket?: WebSocket;
private _url: URL;

onclose?: () => void;
onclose?: () => void | Promise<void>;
onerror?: (error: Error) => void;
onmessage?: (message: JSONRPCMessage) => void;

Expand Down Expand Up @@ -38,8 +38,8 @@ export class WebSocketClientTransport implements Transport {
resolve();
};

this._socket.onclose = () => {
this.onclose?.();
this._socket.onclose = async () => {
await this.onclose?.();
};

this._socket.onmessage = (event: MessageEvent) => {
Expand Down
29 changes: 13 additions & 16 deletions packages/core/src/shared/protocol.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -316,7 +316,7 @@
*
* This is invoked when {@linkcode Protocol.close | close()} is called as well.
*/
onclose?: () => void;
onclose?: () => void | Promise<void>;

/**
* Callback for when an error occurs.
Expand Down Expand Up @@ -456,11 +456,11 @@
async connect(transport: Transport): Promise<void> {
this._transport = transport;
const _onclose = this.transport?.onclose;
this._transport.onclose = () => {
this._transport.onclose = async () => {
try {
_onclose?.();
if (_onclose) await _onclose();
} finally {
this._onclose();
await this._onclose();
}
};

Expand Down Expand Up @@ -490,15 +490,18 @@
await this._transport.start();
}

private _onclose(): void {
private async _onclose(): Promise<void> {
const responseHandlers = this._responseHandlers;
this._responseHandlers = new Map();
this._progressHandlers.clear();
this._taskManager.onClose();
this._pendingDebouncedNotifications.clear();
this._transport = undefined;

await this.onclose?.();

for (const info of this._timeoutInfo.values()) {
clearTimeout(info.timeoutId);

Check failure on line 504 in packages/core/src/shared/protocol.ts

View check run for this annotation

Claude / Claude Code Review

protocol._onclose() drops try/finally, in-flight requests may hang on async onclose rejection 

The refactored `_onclose()` in `protocol.ts` removed the `try/finally` block that previously guaranteed cleanup of in-flight requests even if the `onclose` callback threw. Since this PR introduces async `onclose` callbacks as its primary feature, any rejection from `await this.onclose?.()` will now skip notifying pending response handlers with `ConnectionClosed`, clearing timeout info, and aborting request handler `AbortController`s — causing in-flight requests to hang until their individual tim
Comment on lines +493 to 504
Copy link
Copy Markdown

@claude claude bot Apr 9, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔴 The refactored _onclose() in protocol.ts removed the try/finally block that previously guaranteed cleanup of in-flight requests even if the onclose callback threw. Since this PR introduces async onclose callbacks as its primary feature, any rejection from await this.onclose?.() will now skip notifying pending response handlers with ConnectionClosed, clearing timeout info, and aborting request handler AbortControllers — causing in-flight requests to hang until their individual timeouts expire instead of failing immediately. Fix: wrap the await this.onclose?.() call in a try { ... } finally { /* cleanup */ } block.

Extended reasoning...

The Bug

The _onclose() method was refactored from synchronous to async to support the new async onclose callback. In the original code, the onclose callback was called inside a try { } finally { } block that unconditionally ran cleanup: notifying response handlers with a ConnectionClosed error, clearing timeout handles, and aborting AbortControllers for in-flight server-side request handlers. The new code calls await this.onclose?.() with no try/finally wrapping the subsequent cleanup loops.

The Code Path

In packages/core/src/shared/protocol.ts, _onclose() (starting around line 490 in the modified file):

  1. responseHandlers is captured from this._responseHandlers (line ~492) and the map is cleared.
  2. await this.onclose?.() is called (line ~501) — no try/finally.
  3. The cleanup loops — clearTimeout for all _timeoutInfo entries, handler(error) for each captured responseHandler, and controller.abort(error) for each captured AbortController — only execute if the await resolves successfully.

Why Existing Code Doesn't Prevent It

The outer connect() wrapper does use try/finally to ensure this._onclose() is called even if the old transport's onclose throws. But that only ensures _onclose() runs — it does not protect the code inside _onclose() from a rejection thrown by the user's onclose callback. Once await this.onclose?.() rejects, the async function propagates the rejection and the remaining cleanup statements are skipped.

Impact

If a user supplies an async onclose callback (which is now the advertised feature of this PR) that rejects:

  1. Pending in-flight requests whose handlers are stored in the captured responseHandlers map will never receive the ConnectionClosed error — they will hang until their individual timeouts expire (default 60 seconds).
  2. clearTimeout calls will be skipped, leaking setTimeout handles.
  3. Server-side AbortControllers for active request handlers will not be aborted, so long-running server-side tool handlers will not be cancelled promptly.

Step-by-Step Proof

  1. Client connects and sends a request; protocol._responseHandlers now contains one entry with the pending handler.
  2. User sets protocol.onclose = async () => { await doAsyncCleanup(); /* throws */ }.
  3. Connection drops; transport fires its onclose callback.
  4. connect()'s wrapper calls await this._onclose().
  5. Inside _onclose: responseHandlers is captured (step 1 entry is snapshotted), then await this.onclose?.() rejects.
  6. _onclose() propagates the rejection — the for (const handler of responseHandlers.values()) { handler(error); } loop never executes.
  7. The pending request from step 1 sits unresolved until its 60-second timeout fires, rather than immediately receiving ConnectionClosed.

Fix

Wrap the await this.onclose?.() call in a try/finally inside _onclose():

try {
    await this.onclose?.();
} finally {
    for (const info of this._timeoutInfo.values()) { clearTimeout(info.timeoutId); }
    this._timeoutInfo.clear();
    const requestHandlerAbortControllers = this._requestHandlerAbortControllers;
    this._requestHandlerAbortControllers = new Map();
    const error = new SdkError(SdkErrorCode.ConnectionClosed, 'Connection closed');
    for (const handler of responseHandlers.values()) { handler(error); }
    for (const controller of requestHandlerAbortControllers.values()) { controller.abort(error); }
}

}
this._timeoutInfo.clear();

Expand All @@ -507,18 +510,12 @@

const error = new SdkError(SdkErrorCode.ConnectionClosed, 'Connection closed');

this._transport = undefined;

try {
this.onclose?.();
} finally {
for (const handler of responseHandlers.values()) {
handler(error);
}
for (const handler of responseHandlers.values()) {
handler(error);
}

for (const controller of requestHandlerAbortControllers.values()) {
controller.abort(error);
}
for (const controller of requestHandlerAbortControllers.values()) {
controller.abort(error);
}
}

Expand Down
2 changes: 1 addition & 1 deletion packages/core/src/shared/transport.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -98,7 +98,7 @@ export interface Transport {
*
* This should be invoked when {@linkcode Transport.close | close()} is called as well.
*/
onclose?: () => void;
onclose?: () => void | Promise<void>;

/**
* Callback for when an error occurs.
Expand Down
4 changes: 2 additions & 2 deletions packages/core/src/util/inMemory.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ export class InMemoryTransport implements Transport {
private _messageQueue: QueuedMessage[] = [];
private _closed = false;

onclose?: () => void;
onclose?: () => void | Promise<void>;
onerror?: (error: Error) => void;
onmessage?: (message: JSONRPCMessage, extra?: { authInfo?: AuthInfo }) => void;
sessionId?: string;
Expand Down Expand Up @@ -48,7 +48,7 @@ export class InMemoryTransport implements Transport {
try {
await other?.close();
} finally {
this.onclose?.();
await this.onclose?.();
}
}

Expand Down
24 changes: 18 additions & 6 deletions packages/core/test/inMemory.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -103,8 +103,12 @@ describe('InMemoryTransport', () => {
let clientCloseCount = 0;
let serverCloseCount = 0;

clientTransport.onclose = () => clientCloseCount++;
serverTransport.onclose = () => serverCloseCount++;
clientTransport.onclose = () => {
clientCloseCount++;
};
serverTransport.onclose = () => {
serverCloseCount++;
};

await clientTransport.close();

Expand All @@ -114,7 +118,9 @@ describe('InMemoryTransport', () => {

test('should handle double close idempotently', async () => {
let clientCloseCount = 0;
clientTransport.onclose = () => clientCloseCount++;
clientTransport.onclose = () => {
clientCloseCount++;
};

await clientTransport.close();
await clientTransport.close();
Expand All @@ -126,8 +132,12 @@ describe('InMemoryTransport', () => {
let clientCloseCount = 0;
let serverCloseCount = 0;

clientTransport.onclose = () => clientCloseCount++;
serverTransport.onclose = () => serverCloseCount++;
clientTransport.onclose = () => {
clientCloseCount++;
};
serverTransport.onclose = () => {
serverCloseCount++;
};

await Promise.all([clientTransport.close(), serverTransport.close()]);

Expand All @@ -137,7 +147,9 @@ describe('InMemoryTransport', () => {

test('should fire onclose even if peer onclose throws', async () => {
let clientCloseCount = 0;
clientTransport.onclose = () => clientCloseCount++;
clientTransport.onclose = () => {
clientCloseCount++;
};
serverTransport.onclose = () => {
throw new Error('boom');
};
Expand Down
2 changes: 1 addition & 1 deletion packages/core/test/shared/protocol.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,7 @@ interface TestProtocolInternals {

// Mock Transport class
class MockTransport implements Transport {
onclose?: () => void;
onclose?: () => void | Promise<void>;
onerror?: (error: Error) => void;
onmessage?: (message: unknown) => void;

Expand Down
Loading
Loading