fix: detect parent process death via stdin EOF to prevent MCP server leaks#1353
fix: detect parent process death via stdin EOF to prevent MCP server leaks#1353Blevene wants to merge 1 commit intothedotmack:mainfrom
Conversation
…leaks
When a Claude Code session ends (terminal close, crash, network disconnect),
the spawned mcp-server.cjs process is never cleaned up because:
1. The MCP SDK's StdioServerTransport only listens for 'data' and 'error'
on stdin — never 'end' or 'close'. When the parent dies and stdin gets
EOF, the transport is oblivious.
2. The server only registered SIGTERM/SIGINT handlers. Abrupt session drops
don't deliver signals — the stdin pipe just closes silently.
This adds:
- process.stdin.on('end'/'close') handlers to detect parent death
- transport.onclose as a belt-and-suspenders measure
- isShuttingDown re-entrancy guard to prevent double-exit races
- reason parameter for diagnostic logging
Fixes thedotmack#1351
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
xkonjin
left a comment
There was a problem hiding this comment.
Code Review
Excellent fix for a critical resource leak issue. This PR addresses the problem where MCP server processes would leak indefinitely when the parent Claude Code process exits or crashes.
What's Good
- Root cause analysis: Clear understanding that the MCP SDK's StdioServerTransport doesn't listen for stdin EOF/close events
- Comprehensive coverage: Handles SIGTERM, SIGINT, stdin end, stdin close, and transport close events
- Guard against double-exit: The isShuttingDown flag prevents race conditions
- Clean logging: Includes shutdown reason for debugging
- Backward compatible: Doesn't change existing behavior, just adds missing cleanup
Security Implications
This is a security-related fix (CWE-404: Improper Resource Shutdown):
- Resource leaks can lead to DoS if many MCP servers accumulate
- Each leaked process holds memory and potentially file descriptors
- In containerized or resource-constrained environments, this could exhaust system resources
Testing Considerations
- Test with parent process crash (kill -9)
- Test with graceful shutdown (SIGTERM/SIGINT)
- Verify no orphaned processes remain after parent exits
- Confirm transport.close() path works correctly
Potential Improvements
- Logging level: Consider using logger.debug() instead of logger.info() for the cleanup message to reduce log noise, as process shutdown is expected behavior
- Error handling: The current implementation doesn't log if cleanup fails (though process.exit(0) happens immediately). Consider logging any cleanup errors before exiting
- Reason parameter: The reason parameter is good for debugging, but could expose internal state if logged to external systems. Ensure logs are properly sanitized
Minor Nitpicks
- The comment mentions "without this handler the process leaks indefinitely" - this is correct, but it might be worth noting that the process is now properly guarded against the specific scenario of parent process death via stdin EOF
Summary
This is a solid, well-thought-out fix for a resource leak that could have significant operational impact. The implementation is clean, well-documented, and addresses the root cause comprehensively.
Status: Approved. This is a critical bug fix that should be merged.
Great work on identifying and addressing this issue!
|
Superseded by the embedded Process Supervisor (PR #1370, v10.5.6). MCP server process cleanup is now handled by the process registry's |
Summary
process.stdin.on('end'/'close')handlers to detect when the parent Claude Code process exits, triggering graceful shutdowntransport.onclosecallback as a secondary detection pathisShuttingDownre-entrancy guard to prevent double-exit races when multiple shutdown events fire simultaneouslyreasonparameter tocleanup()for diagnostic loggingProblem
When a Claude Code session ends (terminal close, crash, network disconnect), the spawned
mcp-server.cjsprocess leaks indefinitely because:StdioServerTransportonly listens fordataanderroron stdin — neverendorcloseSIGTERM/SIGINThandlers, which don't fire on abrupt session dropsProcesses accumulate without bound — I found an orphaned instance from 2 days prior still running.
Test plan
mcp-server.cjsspawnsps aux | grep mcp-server)kill -9— verify the MCP server still exits via stdin EOFSIGTERM/SIGINTshutdown still works as beforeFixes #1351
🤖 Generated with Claude Code