Repository navigation
Conversation
The MCP stdio lifecycle has the client shut the server down by closing our stdin, but nothing observed that. StdioServerTransport subscribes only to 'data' and 'error', so EOF raised no event, and the scheduler tick, the hooks rate-limit timer and IMAP IDLE sockets kept the event loop from draining. Any client death that skipped SIGTERM — closed terminal, SIGKILL, crash, MCP reconnect — left an immortal process reparented to launchd/init. 232 such orphans accumulated on one workstation in 7 days, holding 5.2 GB RSS and pushing the machine into 13.3 GB of swap. Shut down on stdin 'end'/'close' and on transport.onclose, add SIGHUP beside the existing signals, and make shutdown idempotent because those triggers routinely fire together. A grace timer forces the exit if a goodbye round trip hangs, so a stuck QUIT/LOGOUT cannot resurrect the orphan. Also guard the post-handshake block: it sits behind two awaits, so a client disconnecting mid-startup could arm the scheduler tick after shutdown had already run, leaving a ref'd handle nothing clears. The regression test spawns a real child process, since the defect is the event loop failing to drain and that is unobservable in-process. Refs codefuturist#60 Co-Authored-By: Claude Opus 5 <[email protected]>
Self-review of the previous commit surfaced a second await boundary. A watcher that finishes starting after shutdown has already run holds IMAP IDLE sockets the completed stop() never saw, so the loop stayed pinned and the grace timer forced exit 1 instead of a clean stop. With the watcher enabled that is not an edge case: reconnect storms hit exactly that window. stop() is idempotent, so the guard re-runs it and bails. Also replace the paraphrase of the shutdown contract with the specification's own wording, which is stronger than the paraphrase implied — the client SHOULD initiate shutdown by "first, closing the input stream to the child process (the server)", then by "waiting for the server to exit" before escalating to SIGTERM. Co-Authored-By: Claude Opus 5 <[email protected]>
Author
|
Closing: this work now lives in Mailoo (https://github.com/Bitfloo/mailoo), an LGPL-3.0 fork that ships it. Thanks for the project. |
Fe2-O3
pushed a commit
to Fe2-O3/email-mcp
that referenced
this pull request
Sep 26, 2026
The MCP spec puts shutdown on the client: over stdio it closes the server's input stream, then waits for the server to exit before escalating to SIGTERM and SIGKILL. Nothing observed EOF, and several services hold ref'd handles (scheduler tick, hooks rate-limit timer, IMAP IDLE sockets), so the event loop never drained on its own and every client death that skipped SIGTERM left an immortal process reparented to launchd. EOF is the primary signal, not a fallback: it is the only one that survives a SIGKILLed client, since the kernel closes the pipe either way, and the only one Windows has. Listens on 'end'/'close' rather than 'data', which would switch stdin to flowing mode and eat protocol bytes from under the SDK. Carries an unref'd grace timer so a hung SMTP QUIT or IMAP LOGOUT cannot resurrect the orphan this prevents, and a re-entrancy guard since EOF, 'close' and a signal routinely arrive together. Ported onto the serveStdio architecture, which owns the transport; three adaptations were required: - serveStdio closes its transport at EOF but closing a transport does not drain the event loop, so the EOF listeners stay necessary. - startBackgroundServices had a startup/shutdown race: stop() could run while the startup block was still awaiting watcher start, the scheduler sweep or the first mirror reconcile, and intervals armed after that check outlived their own teardown and pinned the loop forever. Every resume point in the startup block now re-checks a stopped flag before arming. - mcpLog writes to stderr, so the lifecycle regression test watches stderr for the ready line instead of stdout. Verified by a test that spawns a real child process, completes the handshake, closes stdin immediately after startup completes and asserts a zero exit within budget — the defect is the event loop failing to drain, which is unobservable from inside the same process. Adapted from codefuturist#61 by @majkelooo; his exit-on-close idea, restructured for the factory-based transport and hardened against the startup race it exposes. Upstream: codefuturist#60, codefuturist#61
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Fixes #60 — the stdio server never exited when the client closed its stdin, so every client death that skipped
SIGTERM(closed terminal,SIGKILL, crash, MCP reconnect) left an immortal process reparented tolaunchd/init. I found 232 of them on one workstation after 7 days of uptime, holding 5.2 GB RSS and pushing the machine into 13.3 GB of swap.The spec is explicit about whose job this is. From
basic/lifecycle, Shutdown:"Waiting for the server to exit" is the part this package could not honour. Three things had to coincide, and all three did:
StdioServerTransportsubscribes to'data'and'error'only, so EOF on stdin raises no event andtransport.onclosenever fires in stdio mode.runServerlistened for it either —transport.onclosewas wired on the HTTP path only.This PR shuts down on stdin
'end'/'close'and ontransport.onclose, addsSIGHUPbeside the existing signals, and makesshutdownidempotent because those triggers routinely arrive together. A grace timer forces the exit if a goodbye round trip hangs, so a stuckQUIT/LOGOUTcannot resurrect the orphan.It also guards the post-handshake block, at both of its
awaitboundaries. That block sits behind twoawaits, so a client disconnecting mid-startup could arm the scheduler tick aftershutdownhad already run — a ref'd handle nothing clears. The new test caught this; without the guard it fails with exit code 1 from the grace timer instead of hanging. It is pre-existing (aSIGTERMduring startup did the same) but only becomes observable once the server can exit at all.A second review pass found the same shape one
awaitearlier, atwatcherService.start(): a watcher finishing its start aftershutdownhas run holds IMAP IDLE sockets the completedstop()never saw, so the loop stays pinned and the grace timer forces exit 1 rather than a clean stop. With the watcher enabled that is not an edge case — reconnect storms hit precisely that window.stop()is idempotent, so the guard re-runs it and bails.Reproduction
Minimal, no account needed — this isolates the mechanism:
printf 'x\n' | node a.mjsexits on its own;printf 'x\n' | node b.mjsnever does.For a control:
chrome-devtools-mcpruns through the samenpxlauncher and the same SDK on the same machine and left zero orphans over those 7 days. The launcher and the SDK are not the differentiator; the event-loop-pinning handle is.Decisions worth a second opinion
.unref(). Unref'ing the scheduler interval alone fixes nothing, becausehooks.service.ts(rateResetTimer) and IMAP IDLE sockets still pin the loop. Unref'ing all of them would touch three services and still leave the loop pinned by any handle added later.shutdown()already stops every one of them, so the missing piece was only ever the trigger. Happy to add.unref()as belt-and-braces if you prefer it.process.exitin the grace timer needs ann/no-process-exitdisable.process.exitCodeis the house style, but it only takes effect once the loop drains — and a hung shutdown is exactly the case where it will not. The reasoning is in a comment at the call site.'data'listener on stdin, deliberately: it would switch the stream to flowing mode and consume protocol bytes out from under the SDK.'end'/'close'leave the mode untouched.transport.oncloseis redundant today and wired anyway, so this becomes the idiomatic path if the SDK ever emits it. Worth knowing: Bug: StdioServerTransport doesn't handle stdin close/end — causes zombie process accumulation modelcontextprotocol/typescript-sdk#2002 reports the SDK-side gap and has had three competing PRs open since May with none merged. Fixing the SDK alone would not have fixed this package anyway, given point 2 above.Test
src/main.lifecycle.test.tsspawns the server as a real child process — the defect is the event loop failing to drain, which is unobservable in-process. It completes the handshake, closes stdin, and asserts a clean exit. Everything is confined to a temp dir viaXDG_*, so it cannot see a developer's real config or touch a real scheduled-mail queue, andafterEachkills the child — a test about leaked processes should not leak one.Verified red before / green after by stashing the
src/main.tschange:Type of change
Checklist
pnpm checkand it passespnpm typecheckand it passespnpm test: 151 passed / 16 filesOne behavioural consequence worth stating explicitly: a server spawned with
stdio: 'ignore'gets/dev/nullas stdin and therefore an immediate EOF, so it now exits at once. For a stdio transport that seems correct — such a server has no client and can never receive a request — but it is a change from today's silent hang.