fix(server): exit when MCP client closes stdin pipe#2003
fix(server): exit when MCP client closes stdin pipe#2003ElliotDrel wants to merge 2 commits intomodelcontextprotocol:mainfrom
Conversation
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
|
@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:
|
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