Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
7 changes: 7 additions & 0 deletions .changeset/stdio-server-stdin-eof-close.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
---
'@modelcontextprotocol/sdk': patch
---

`StdioServerTransport` now closes itself and fires `onclose` when its stdin ends or closes, so a server whose client hangs up tears down through `onclose` and can exit, instead of running on with no parent. A stdin that had already ended or been destroyed before `start()` is
treated the same way. `close()` is idempotent, so `onclose` fires once when `close()` is also called. Requests still in flight when stdin ends are aborted and not answered, and a notification sent after that rejects with `Not connected`; a client that expects responses keeps
stdin open until it has read them. An `onclose` that calls `process.exit()` can cut off output that is still being written.
21 changes: 21 additions & 0 deletions src/__fixtures__/serverWithKeepAlive.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
import { setInterval } from 'node:timers';
import { McpServer } from '../server/mcp.js';
import { StdioServerTransport } from '../server/stdio.js';

const transport = new StdioServerTransport();

const server = new McpServer({
name: 'server-with-keep-alive',
version: '1.0.0'
});

await server.connect(transport);

// Simulates a real server holding a keep-alive handle (connection pool, file
// watcher, heartbeat timer, ...). A well-behaved server releases its handles
// when the connection closes, and the process then exits naturally.
const keepAlive = setInterval(() => {}, 60_000);

server.server.onclose = () => {
clearInterval(keepAlive);
};
30 changes: 30 additions & 0 deletions src/server/stdio.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,10 +8,16 @@ import { Transport } from '../shared/transport.js';
* Server transport for stdio: this communicates with an MCP client by reading from the current process' stdin and writing to stdout.
*
* This transport is only available in Node.js environments.
*
* When the client closes its end of the pipe (stdin reaches end-of-file), the transport
* closes itself and fires `onclose`. A server that holds no other keep-alive handles will
* then exit naturally. Requests still in flight when stdin ends are not answered; a client
* that expects responses keeps stdin open until it has read them.
*/
export class StdioServerTransport implements Transport {
private _readBuffer: ReadBuffer;
private _started = false;
private _closed = false;

constructor(
private _stdin: Readable = process.stdin,
Expand Down Expand Up @@ -46,6 +52,16 @@ export class StdioServerTransport implements Transport {
_onerror = (error: Error) => {
this.onerror?.(error);
};
_onstdinclose = () => {
// stdin EOF means the client hung up and nothing more can arrive.
const open = !this._closed && !this._stdin.destroyed;
this.close().catch(() => {});
// A socket can still fail a pending write after 'end': keep reporting errors until it closes.
if (open) {
this._stdin.on('error', this._onerror);
this._stdin.once('close', () => this._stdin.off('error', this._onerror));
}
};
Comment on lines +55 to +64

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🔴 Scripted callers that pipe requests in and close stdin (half-close, e.g. printf '…' | node server.js) now get no response for any handler still running at EOF. src/server/stdio.ts:52 closes the transport on 'end', and Protocol._onclose (src/shared/protocol.ts:657) aborts every in-flight handler, so its response is skipped at src/shared/protocol.ts:788. The changeset calls this accepted; in fact it hits every one-shot pipe whose tool handler awaits anything, since the writer closes the pipe immediately. Fix: on stdin 'end' (as opposed to 'close'/'error'), let request handlers already received before EOF finish and write their responses to stdout before onclose fires and handlers are aborted; keep immediate close on destroy. A Protocol-level drain is the durable fix.

Why this was flagged

A client or shell script writes one or more requests to the server's stdin and then closes its write end while still reading stdout, e.g. printf '…' | node server.js with a tool handler that awaits file or network I/O. The data chunk arrives, Protocol._onrequest schedules the handler, then the EOF read callback fires 'end' and src/server/stdio.ts:73 invokes _onstdinclose, which calls close() and onclose synchronously (src/server/stdio.ts:114). Protocol._onclose aborts every controller in _requestHandlerAbortControllers (src/shared/protocol.ts:657). When the handler resolves, src/shared/protocol.ts:788 sees signal.aborted and returns without sending the JSON-RPC response. On the base branch the transport ignored 'end', the handler completed and the response was written to stdout; after this change the caller reads nothing on stdout and the request silently vanishes. The new test 'should still deliver messages that arrived before stdin ended' only asserts onmessage was called, not that a response is written, so it does not catch this.

Verification: src/server/stdio.ts:73 registers this._stdin.on('end', this._onstdinclose), and close() invokes this.onclose?.() at line 114. Protocol's _onclose at src/shared/protocol.ts:657-660 aborts every in-flight handler; lines 788-791 then return on abortController.signal.aborted, so no response is written to stdout. On c2ce003 start() only listened for 'data' and 'error', so the response was written.


/**
* Starts listening for messages on stdin.
Expand All @@ -58,8 +74,15 @@ export class StdioServerTransport implements Transport {
}

this._started = true;
// A stream that already ended or was destroyed will never emit 'end'/'close' again.
// Deferred so an onclose assigned right after connect() still sees it.
if (this._stdin.readableEnded || this._stdin.destroyed) {
setImmediate(this._onstdinclose);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟡 (optional) Servers that reconnect a fresh transport from onclose now spin forever at 100% CPU once the client hangs up, where the base left them idle. src/server/stdio.ts:68-69 sees process.stdin already readableEnded on every new start() and schedules _onstdinclose via setImmediate, which closes, fires onclose, and the handler connects again, with no yield between iterations. Fix: do not re-arm a dead stdin on each start(); either throw from start() when stdin is already ended/destroyed, or fire onclose once per stream and refuse further starts on the same ended stream.

Why this was flagged

Trigger: user code that resets the server on close, e.g. server.onclose = () => server.connect(new StdioServerTransport()). Entry point is onclose fired from close() at src/server/stdio.ts:114 after stdin EOF. On the second connect, start() at src/server/stdio.ts:68 finds process.stdin.readableEnded true and calls setImmediate(this._onstdinclose) at line 69; that runs close() (line 92), which fires onclose (line 114), which connects again, and so on. setImmediate callbacks run every event-loop turn with no delay, so the process busy-loops and never exits. On the base branch the second transport only attached 'data'/'error' listeners to an ended stream and nothing fired, so the process sat idle and exited when its other handles were released. Remedy: make start() on an already-ended or destroyed stdin reject (or fire onclose only once per Readable, tracked on the stream), so a reconnect loop terminates with an error instead of spinning.

Verification: src/server/stdio.ts:68-69 fires setImmediate(this._onstdinclose) on every start() against an already-ended stdin; _onstdinclose calls close(), which calls this.onclose?.() at line 114. Protocol's wrapper sets this._transport = undefined at src/shared/protocol.ts:664 before invoking the user's onclose at 665, so the reconnect passes the already-connected guard and reaches start() again, with no terminating condition.

}
this._stdin.on('data', this._ondata);
this._stdin.on('error', this._onerror);
this._stdin.on('end', this._onstdinclose);
this._stdin.on('close', this._onstdinclose);
}

private processReadBuffer() {
Expand All @@ -78,9 +101,16 @@ export class StdioServerTransport implements Transport {
}

async close(): Promise<void> {
if (this._closed) {
return;
}
this._closed = true;

// Remove our event listeners first
this._stdin.off('data', this._ondata);
this._stdin.off('error', this._onerror);
this._stdin.off('end', this._onstdinclose);
this._stdin.off('close', this._onstdinclose);

// Check if we were the only data listener
const remainingDataListeners = this._stdin.listenerCount('data');
Expand Down
45 changes: 45 additions & 0 deletions test/integration-tests/processCleanup.test.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import { spawn } from 'node:child_process';
import path from 'node:path';
import { Readable, Writable } from 'node:stream';
import { Client } from '../../src/client/index.js';
Expand Down Expand Up @@ -76,6 +77,50 @@ describe('Process cleanup', () => {
expect(onCloseWasCalled).toBe(1);
});

it('server process should exit on its own when the client closes stdin', async () => {
// Regression test for zombie stdio servers: when the client drops its end of
// the pipe (window closed, session restarted) without sending any signal, the
// server must notice stdin EOF, close its transport, and fire `onclose` so the
// server can release its keep-alive handles and the process exits naturally.
const child = spawn('node', ['--import', 'tsx', 'serverWithKeepAlive.ts'], {
cwd: FIXTURES_DIR,
stdio: ['pipe', 'pipe', 'inherit']
});

const exited = new Promise<number | null>(resolve => {
child.on('exit', code => resolve(code));
});

// Confirm the server is up by completing an initialize round-trip over raw stdio.
const initialized = new Promise<void>(resolve => {
child.stdout.on('data', () => resolve());
});
child.stdin.write(
JSON.stringify({
jsonrpc: '2.0',
id: 1,
method: 'initialize',
params: {
protocolVersion: '2025-06-18',
capabilities: {},
clientInfo: { name: 'stdin-eof-test', version: '1.0.0' }
}
}) + '\n'
);
await initialized;

// Hang up: close our end of the pipe without any signal.
child.stdin.end();

// The server must exit on its own — no SIGTERM/SIGKILL involved. Bound the
// wait so a regression fails the test instead of leaking the child.
const exitCode = await Promise.race([exited, new Promise<'zombie'>(resolve => setTimeout(() => resolve('zombie'), 8000))]);
if (exitCode === 'zombie') {
child.kill('SIGKILL');
}
expect(exitCode).toBe(0);
}, 15_000);

it('should exit cleanly for a server that hangs', async () => {
const client = new Client({
name: 'test-client',
Expand Down
Loading
Loading