fix(server): unlink legacy .server.pid on atexit and SIGTERM (closes #437)#439
Merged
fix(server): unlink legacy .server.pid on atexit and SIGTERM (closes #437)#439
Conversation
…437) Previously ~/.memtomem/.server.pid was held via fcntl.flock for the process lifetime but the file was never deleted on shutdown. The flock itself is released by kernel cleanup when the process dies, so a single next invocation would succeed — but parallel MCP health probes (e.g. `claude mcp list` scanning multiple servers) can race on `open("a+b") + flock(LOCK_EX|LOCK_NB)` against the stale file, hitting the "another memtomem-server holds a lock (likely a pre-0.1.25 install)" abort branch. The user-visible symptom is a transient "Failed to connect" in Claude Code that recovers only after manual `rm ~/.memtomem/.server.pid`. This mirrors the teardown already in place for the new `$XDG_RUNTIME_DIR/memtomem/server.pid`: - `atexit.register` gains an `unlink(missing_ok=True)` for the legacy path when `_try_hold_legacy_flock` succeeded (LIFO-ordered before the flock close so the unlink still targets an owned file). - `_install_sigterm_handler` is now variadic (`*pid_files`) and `main()` passes both the new and legacy pid files so SIGTERM cleans up both. Tested: - New unit test (`test_sigterm_handler_unlinks_all_pid_files`) pins the variadic-unlink contract. - New end-to-end test (`test_sigterm_unlinks_legacy_pid_file_end_to_end`) spawns `memtomem-server` with an existing `~/.memtomem/`, sends SIGTERM, and asserts the legacy pid file is gone. Would fail without this change. - Existing `test_server_refuses_when_legacy_lock_held` still passes (unchanged: when a real holder has the flock, new server must abort). - Full CI-filter suite (`-m "not ollama"`): 2251 passed. Co-Authored-By: Claude <[email protected]>
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 subscribe to this conversation on GitHub.
Already have an account?
Sign in.
2 participants
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.
Summary
~/.memtomem/.server.pidnow unlinks on bothatexit(normal stdin-EOF exit) and SIGTERM, matching the teardown the new$XDG_RUNTIME_DIR/memtomem/server.pidpath already has._install_sigterm_handlerbecomes variadic (*pid_files: Path) somain()can pass the new and legacy pid files together when_try_hold_legacy_flockacquired the legacy lock.Fixes #437. Root cause investigation and live repro are in the issue comments.
The bug surfaces as
✘ Failed to connectin Claude Code /claude mcp listafter any priormemtomem-servershutdown, because the legacy file remains on disk and parallel MCP probes race onopen("a+b") + flock(LOCK_EX|LOCK_NB), then exit via the "likely a pre-0.1.25 install" branch.rm ~/.memtomem/.server.pidwas the manual workaround.Why variadic instead of two separate calls
Two separate
signal.signal(SIGTERM, ...)calls would have the second overwrite the first — only the last-registered handler fires. Bundling both paths into one handler is the shape that keeps the invariant ("SIGTERM cleans up every pid file this process owns").Test plan
uv run pytest packages/memtomem/tests/test_server_sigterm.py -v— 7 passedtest_sigterm_handler_unlinks_all_pid_files(variadic unit)test_sigterm_unlinks_legacy_pid_file_end_to_end(subprocess + SIGTERM + legacy pid unlink assertion)uv run pytest packages/memtomem/tests/ -m "not ollama"— 2251 passeduv run ruff check packages/memtomem/src && uv run ruff format --check packages/memtomem/srcuv run mypy packages/memtomem/src/memtomem/server/__init__.py— no issuestest_server_refuses_when_legacy_lock_heldstill passes (unchanged contract: when a real holder has the flock, new server must still abort)Out of scope
_detect_project_installdoesn't verifymemtomemis in project dependencies #436) that also contributed to the original live report (mm initproducing a workspace-form.mcp.jsonfor an emptyuv initproject).🤖 Generated with Claude Code