fix(server): exit when MCP client closes stdin pipe#2003
Conversation
🦋 Changeset detectedLatest commit: a3060eb The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
@modelcontextprotocol/client
@modelcontextprotocol/server
@modelcontextprotocol/express
@modelcontextprotocol/fastify
@modelcontextprotocol/hono
@modelcontextprotocol/node
commit: |
There was a problem hiding this comment.
Pull request overview
Fixes orphaned/zombie StdioServerTransport server processes by detecting when the MCP client disconnects from stdin and initiating transport shutdown.
Changes:
- Add a stdin
closelistener inStdioServerTransport.start()that triggersclose(). - Remove the stdin
closelistener duringStdioServerTransport.close()cleanup. - Add tests intended to assert
onclosefires on stdin termination signals.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| packages/server/src/server/stdio.ts | Adds stdin close handling to trigger transport shutdown and cleans up the listener on close. |
| packages/server/test/server/stdio.test.ts | Adds tests for stdin close/end behavior and ensuring onclose isn’t double-fired. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| // Remove our event listeners first | ||
| this._stdin.off('data', this._ondata); | ||
| this._stdin.off('error', this._onerror); | ||
| this._stdin.off('close', this._onstdinclose); | ||
| this._stdout.off('error', this._onstdouterror); | ||
|
|
There was a problem hiding this comment.
close() cleans up the newly added stdin close handler, but if you add an end handler for stdin (needed for reliable pipe-disconnect detection), it should also be removed here to avoid leaking listeners across start/close cycles.
| const server = new StdioServerTransport(input, output); | ||
| server.onerror = error => { throw error; }; | ||
|
|
||
| let closeCount = 0; | ||
| server.onclose = () => { closeCount++; }; | ||
|
|
||
| await server.start(); | ||
| input.push(null); // signals end-of-stream | ||
|
|
||
| // Allow microtasks to flush | ||
| await new Promise(resolve => setTimeout(resolve, 0)); | ||
|
|
There was a problem hiding this comment.
This test claims to validate behavior on stdin end, but input.push(null) may also trigger a close event depending on stream settings, so it can pass even if the transport doesn't handle end at all. Make the test explicitly verify the end path (e.g., emit end without close, or construct a Readable where emitClose/autoDestroy won't emit close on end) so it fails unless an end listener is implemented.
| const server = new StdioServerTransport(input, output); | |
| server.onerror = error => { throw error; }; | |
| let closeCount = 0; | |
| server.onclose = () => { closeCount++; }; | |
| await server.start(); | |
| input.push(null); // signals end-of-stream | |
| // Allow microtasks to flush | |
| await new Promise(resolve => setTimeout(resolve, 0)); | |
| const endOnlyInput = new Readable({ | |
| autoDestroy: false, | |
| emitClose: false, | |
| // We'll use endOnlyInput.push() instead. | |
| read: () => {} | |
| }); | |
| const server = new StdioServerTransport(endOnlyInput, output); | |
| server.onerror = error => { throw error; }; | |
| let closeCount = 0; | |
| let inputCloseCount = 0; | |
| server.onclose = () => { closeCount++; }; | |
| endOnlyInput.on('close', () => { inputCloseCount++; }); | |
| await server.start(); | |
| endOnlyInput.push(null); // signals end-of-stream without emitting close | |
| // Allow microtasks to flush | |
| await new Promise(resolve => setTimeout(resolve, 0)); | |
| expect(inputCloseCount).toBe(0); |
| this._started = true; | ||
| this._stdin.on('data', this._ondata); | ||
| this._stdin.on('error', this._onerror); | ||
| this._stdin.on('close', this._onstdinclose); | ||
| this._stdout.on('error', this._onstdouterror); |
There was a problem hiding this comment.
start() only listens for stdin's close event, but a disconnected pipe commonly emits end (and close is not guaranteed for all Readable streams). To reliably shut down on client disconnect, also listen for stdin's end event and remove that listener in close() alongside the existing close cleanup.
|
Addressed all three Copilot review comments:
|
|
Added a changeset for patch-level bump on |
|
All Copilot review feedback has been addressed in the latest commits:
CI is all green. Ready for maintainer review. |
|
Checking in — CI is green across all targets (Node 20/22/24, Bun, Deno) and all three rounds of Copilot review feedback have been addressed. Tagging for a maintainer review pass when someone has a moment. |
|
Hey @felixweinberger and @KKonstantinov — this PR is ready for a human review pass when you get a chance. Quick summary: fixes unbounded zombie process accumulation in No rush — just wanted to make sure it's on your radar. Thanks! |
|
Hitting this on my workstation with a different MCP server (obsidian-mcp) and Evidence (2026-05-13):
Confirmed the fix pattern works: As a stopgap I applied a 1-line vendor patch ( +1 for merge — addresses a real leak class. |
|
Verified this patch works against a real-world third-party MCP server. Applied the equivalent of this PR's change (
Environment: macOS 15, Node 24.14.1, Claude Code (stdio transport). Confirming the fix matches my local patch exactly. Every Node-based MCP server in the wild is affected by this bug — would love to see this land. 🙏 |
|
One regression angle I would add from running MCP servers under long-lived agent sessions: treat stdin EOF as an authority boundary, not just stream cleanup. Beyond |
|
@Christian-Sidak — tagging you as an active maintainer. This PR has been CI-green for 16 days and received independent validation from three community members today (see comments from @waypointmini-rgb, @louis198617, and @carltonawong). The fix is minimal: add |
When an MCP client (e.g. Claude Code) closes its end of the stdin pipe, StdioServerTransport does not detect the disconnect and the server process lingers indefinitely as a zombie/orphan. This adds belt-and-suspenders `process.stdin` close/end listeners immediately after `session.connect(transport)` for the stdio transport path. When either event fires, `transport.close()` is called so the process exits cleanly. The upstream SDK is tracking the same root cause in modelcontextprotocol/typescript-sdk#2003. Adding the fix here means FastMCP users are protected regardless of which SDK version they have installed.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
27f0874 to
a3060eb
Compare
…ocesses (#265) * fix(stdio): add stdin close/end listeners to prevent zombie processes When an MCP client (e.g. Claude Code) closes its end of the stdin pipe, StdioServerTransport does not detect the disconnect and the server process lingers indefinitely as a zombie/orphan. This adds belt-and-suspenders `process.stdin` close/end listeners immediately after `session.connect(transport)` for the stdio transport path. When either event fires, `transport.close()` is called so the process exits cleanly. The upstream SDK is tracking the same root cause in modelcontextprotocol/typescript-sdk#2003. Adding the fix here means FastMCP users are protected regardless of which SDK version they have installed. * style: run prettier on FastMCP.ts * fix: address Copilot review — idempotency, listener cleanup, and stdio test - Add stdinClosed guard to prevent double-close race when both close and end fire for the same shutdown - Remove stdin listeners inside the handler (on first fire) and also in transport.onclose to prevent listener accumulation across start() calls - Add src/FastMCP.stdio.test.ts with vitest tests verifying registration, single-fire idempotency, and listener removal Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * fix: resolve ESLint sort-objects and no-unused-vars in stdio test * fix(lint): remove unused params from stdinOffSpy mockImplementation * fix(lint): apply prettier formatting to stdio test file * fix(test): hoist vi.mock to module level to fix Vitest hoisting issue vi.mock() is statically hoisted by Vitest, so factory closures cannot reference test-body locals. Move fakeTransport to module scope and assign it in beforeEach so the mock factory captures the right value. * fix(test): use fake timers to skip FastMCPSession capability retry loop session.connect() retries getClientCapabilities() up to 10x100ms before resolving. Without fake timers, the test's single Promise.resolve() tick was not enough for the stdin listeners to be registered. * fix(test): use regular function in vi.fn mock so new StdioServerTransport() works Arrow functions cannot be used as constructors. Switching to a regular function in the vi.mock factory lets `new StdioServerTransport()` in FastMCP.start() return fakeTransport correctly. Also removes fake timer usage — the 10×100ms capability-retry loop in session.connect() runs in real time (~1s), and vi.waitFor with a 3s timeout waits it out cleanly. * test: add integration test for stdin-close zombie prevention Spawns a real FastMCP stdio server child process, destroys stdin to simulate client disconnect, and asserts the child exits cleanly. Regression test for #264. * style: fix prettier formatting in stdio integration test * fix(lint): fix perfectionist ordering in stdio integration test Three ESLint perfectionist violations: - node:child_process import before vitest - env before stdio in spawn options object - null before number in union type Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * fix(test): increase timeout for tsx cold-download in CI (60s ready, 90s vitest) * fix(test): add tsx devDep, use installed binary instead of npx cold-download * fix: update pnpm-lock.yaml after adding tsx devDependency * fix(test): use temp .ts file instead of --eval so ESM imports resolve via tsx * style: fix prettier formatting in integration test * fix(lint): sort named imports, add comment to empty catch blocks * revert: remove integration test and tsx devDep Unit tests in FastMCP.stdio.test.ts already cover the fix. The integration test consistently failed due to tsx --eval ESM incompatibility. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> --------- Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com> Co-authored-by: Frank Fiegel <frank@glama.ai>
Problem
StdioServerTransportlistens fordataanderroron stdin, but not forcloseorend. When an MCP client (e.g. Claude Code) closes its window or restarts, it drops its end of the stdio pipe. The server process never detects this and keeps running indefinitely.This causes unbounded zombie process accumulation — every time the client restarts, a new server is spawned, and the old one never dies. Observed in production: 37 orphaned processes consuming 26,000+ CPU-seconds, machine became unresponsive.
This is especially severe on Windows, where
SIGTERMis not reliably delivered to child processes when a parent exits, making stdin close the only cross-platform shutdown signal.Related to #1568 (which fixed stdout EPIPE) but stdin close was left unhandled.
Tracked in issue #2002.
Fix
Add a
closelistener on stdin instart()that callsthis.close(), using the same arrow-function pattern as_onstdouterrorfor proper cleanup onclose().Tests
should fire onclose when stdin emits closeshould fire onclose when stdin emits endshould not fire onclose twice when close() called after stdin close