Repository navigation
fix: exit on stdin EOF so a dead client cannot orphan the server - #83
lzrong0203 wants to merge 1 commit into
Conversation
StdioServerTransport only subscribes to stdin's 'data'/'error' events, so EOF (the client exiting) is never surfaced, and the Discord gateway WebSocket keeps the event loop alive: the server outlives its client indefinitely. Listen for stdin 'end'/'close' at the entry point and shut down cleanly; shutdown() is guarded so the 'end'+'close' pair (or a concurrent signal) cannot double-run it.
|
Thanks for the careful work on this. I checked everything myself: the orphan reproduces on main, and your regression test really does catch it. Your SDK finding is right too. I did hit one problem with the hard |
There was a problem hiding this comment.
Great PR. Real bug, honest scope, and a test that actually catches it. Two things before merging.
1. The hard exit drops responses when input is piped.
Pipe initialize, initialized, tools/list, tools/call, then EOF, with stdout on a pipe:
| bytes | valid responses | |
|---|---|---|
| main | 118,745 | 3 |
| this branch | 65,685 | 1 |
The tools/call reply is never written, and tools/list is cut mid-JSON at the 64 KB pipe buffer. Both exit 0, so nothing warns the caller.
One direction: on EOF stop reading, close the transport, call discord.destroy(), and let the event loop finish pending work. Keep a short unref'd timer as a hard-exit backstop so your orphan fix still holds.
2. The tests cannot run on Windows.
--importwith a raw absolute path throwsERR_UNSUPPORTED_ESM_URL_SCHEME. UsepathToFileURL(KEEPALIVE).href.- The SIGTERM test cannot pass:
killis TerminateProcess emulation, so the handler never runs. Needs{ skip: process.platform === "win32" }.
The runtime fix itself is fine cross-platform.
Nit, not blocking: startServer() discards stderr and has no 'exit' listener, so a boot crash just looks like a 5 second hang.
Happy to merge once 1 and 2 are in. Thanks again.
PaSympa
added
bug
Problem
When the MCP client that spawned
discord-mcpexits, the server process can keep running indefinitely:StdioServerTransportsubscribes only to stdin'sdata/errorevents, so EOF is never surfaced — and once any active handle exists (in practice the Discord gateway WebSocket, once a tool call has logged in), the event loop never drains.Field report: I found 12 orphaned
discord-mcpprocesses on one machine, accumulated over ~3 weeks, each spawned by an MCP client that had long since exited, holding 0.8–2.1 GB RSS between them. (Those long-lived orphans also showed sustained high CPU and did not respond to SIGTERM. I could not reproduce those two symptoms in a controlled setting, so this PR claims to fix only the lingering-process defect — the CPU/RSS/SIGTERM observations are reported for context, not explained by this patch.)Repro
DISCORD_TOKEN=placeholder node --import ./test/fixtures/keepalive.mjs --import tsx src/index.ts < /dev/null< /dev/nulldelivers immediate stdin EOF; the preload stands in for the gateway handle. Onmasterthe process stays alive until killed; with this patch it exits 0 immediately.Fix
src/index.ts: listen for stdinend/closeand shut down cleanly.shutdown()is guarded since EOF deliversendfollowed byclose.Tests
test/lifecycle.test.tsboots the real entry point with the keepalive preload (a referenced timer — login is lazy, so no real gateway opens in tests, and a timer needs no network permissions):master(child still alive at deadline), passes with the fix; assertscode === 0and no signal killmastertoo)Full suite (56 tests),
tsc,eslint, andprettier --checkall pass.Note on the SDK
Arguably the root defect lives in
@modelcontextprotocol/typescript-sdk: theTransportcontract documentsoncloseas firing when the connection closes "for any reason", yetStdioServerTransportnever surfaces stdin EOF (checked against currentmain). This entry-point guard is worth having regardless of an eventual SDK fix; happy to file the SDK issue separately if useful.