mimir mcp: exit when the client dies, not when stdin says so - #16
Merged
Conversation
Fourteen orphaned `mimir mcp` processes had accumulated on a real machine, the oldest fourteen days old, ~2.9 GB resident between them, each holding the 651 MB SQLite database open. The reported hypothesis was that the serve loop treats a zero-length read as "no data yet". It does not. rmcp's transport returns `None` on `Ok(0)` (async_rw.rs:129), the service task ends, `waiting()` resolves. Measured: close stdin and the process exits in 0.3 s, rc=0, repeatably. A fix aimed at EOF handling would have changed nothing. The EOF never arrives. `/proc/<pid>/fd/0` on a live server is `socket:[...]`, not a pipe — MCP stdio is an AF_UNIX socketpair, and a socketpair delivers EOF only once EVERY descriptor for the peer end is closed. Any unrelated process that inherited it — a background command, another server from the same client — pins it open, so when the client exits the kernel delivers nothing and the reader blocks forever. Reproduced 2/2 with a deliberately leaked descriptor, with the leaker's fd verified pointing at the same socket inode. So the trigger cannot be a descriptor. It has to be the parent dying: PR_SET_PDEATHSIG on Linux, a getppid() poll for macOS and for the arming race, SIGTERM/SIGHUP through the same path. Cancelling rmcp's token ends the service task and resolves the `waiting()` already parked on, so shutdown runs the same course as a clean EOF and the engine — and its database handle — drops on the way out. Two things the tests caught that review would not have: - The ppid snapshot must be taken in `main`, not at the watchdog. The server spends a second or two loading models; a client that exits during that window is already gone, so a ppid read there returns the reaper, compares equal to itself forever, and the watchdog never fires. - The watchdog must be armed BEFORE `serve()`, which awaits the MCP `initialize` handshake and blocks indefinitely against a client that spawns the server and dies without initializing. Armed afterwards, that entire window is unguarded — and it is the window the regression test happened to exercise, which is the only reason it was found. Both were live bugs in the first two versions of this fix. Each test was run against a disabled watchdog to confirm it fails without it; the parent-death test also had to stop redirecting the server's stdin from /dev/null (a non-interactive shell does that to background jobs), which had been handing it an instant EOF and making it pass against an unfixed binary. Residual, deliberately not papered over: a parent that dies in the microseconds between exec and `record_parent_pid` still cannot be detected. That is not the failure that produced these orphans. `--http` untouched, request handling untouched, no idle timeout. fmt, clippy -D warnings, 389 tests; manually verified that closing stdin and killing the parent both leave nothing behind.
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.
Fourteen orphaned
mimir mcpprocesses had accumulated on a real machine —oldest fourteen days, ~2.9 GB resident between them, every one holding the
651 MB SQLite database open.
The reported hypothesis was wrong
The suspicion was that the serve loop treats a zero-length read as "no data
yet, keep going". It does not. rmcp's transport returns
NoneonOk(0)(
async_rw.rs:129), the service task ends,waiting()resolves. Measured:close stdin and the process exits in 0.3 s, rc=0, repeatably. A fix aimed
at EOF handling would have changed nothing.
The EOF never arrives
/proc/<pid>/fd/0on a live server issocket:[...], not a pipe — MCP stdiois an AF_UNIX socketpair, and a socketpair delivers EOF only once every
descriptor for the peer end is closed. Any unrelated process that inherited it
pins the server open forever. Reproduced 2/2 with a deliberately leaked
descriptor, the leaker's fd verified pointing at the same socket inode.
Thread census of a stuck server (29 threads) matches: 25
futex_wait(idletokio workers plus main parked in
block_on), 1epoll_wait, 2hrtimer_nanosleep, and exactly one inunix_stream_read_generic— the readthat never returns.
futex_waiton the main thread is not evidence of a bug;that is just
block_on.The fix
The trigger cannot be a descriptor, so it is the parent dying:
PR_SET_PDEATHSIGon Linux, agetppid()poll for macOS and the arming race,SIGTERM/SIGHUP through the same path. Cancelling rmcp's token resolves the
waiting()already parked on, so shutdown takes the same course as a cleanEOF — in-flight drains, transport closes, engine and DB handle drop.
Two bugs the tests caught in the fix itself
Both would have shipped as a fix that fixed nothing:
main. The server spends 1–2 sloading models; a client that exits in that window is already gone, so a
ppid read at the watchdog returns the reaper, compares equal to itself
forever, and it never fires.
serve(), which awaits theinitializehandshake and blocks indefinitely against a client that spawnsthe server and dies without initializing.
The test also had to stop letting the shell redirect the server's stdin from
/dev/null(non-interactive shells do that to background jobs), which washanding it an instant EOF and making it pass against an unfixed binary.
Verification
Every test was run against a disabled watchdog to confirm it fails without it:
parent-death fails at 19 s with "server survived its parent", passes in 4.1 s
with. Manually confirmed nothing is left behind after closing stdin and after
killing the parent.
Residual gap, deliberately not papered over: a parent dying in the
microseconds between
execandrecord_parent_pidstill cannot be detected.That is not the failure that produced these orphans.
--httpuntouched —mimir daemonhas its own lifetime and arms none ofthis. Request handling untouched. No idle timeout. fmt, clippy
-D warnings,389 tests.