Add watcher preopen controls and harden server shutdown#120
Open
isham703 wants to merge 1 commit into
Open
Conversation
STRd6
pushed a commit
to STRd6/mcp-language-server
that referenced
this pull request
May 14, 2026
When the LSP registers workspace/didChangeWatchedFiles patterns, the old behavior spawned a goroutine that walked the entire workspace and sent didOpen for every matching file. The TODO said this was for typescript-language-server, but empirically TS does not register watchers at all (neither do gopls, clangd, civet-lsp). Only rust-analyzer registers them today, and it indexes via cargo metadata independently of didOpen state. For each LSP in our integration-test matrix, all features (definition, references, hover, rename, document symbols, diagnostics) work with the scan removed. Tools that need a specific file already call OpenFile on demand, so the LSP gets a didOpen for the files actually being queried. Eliminates the symptom behind issue isaacphi#83 ("too many open files") and the corresponding memory blowup on large rust workspaces, without needing PR isaacphi#120's runtime flags. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
STRd6
pushed a commit
to STRd6/mcp-language-server
that referenced
this pull request
May 14, 2026
cleanup() in main.go can fire from three independent goroutines: the signal handler (SIGINT/SIGTERM), the parent-death watcher, and the main goroutine's error-exit path. Today each call independently drives CloseAllFiles -> Shutdown -> Exit -> Close on the LSP client, which produces two parallel teardowns when triggers fire close in time -- duplicate didClose notifications, racing access to s.lspClient, and occasional shutdown-after-exit log noise. Wrap the body of cleanup in a package-level sync.Once and lift lsp.Client.Close into a sync.Once.Do that caches its return value in c.closeErr. The first caller runs the teardown; subsequent callers (and the forthcoming idle watchdog) block on the Once until it completes, then return the cached error without doing the work twice. Adapted from upstream PR isaacphi#120 (isham703). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
STRd6
pushed a commit
to STRd6/mcp-language-server
that referenced
this pull request
May 14, 2026
Catches the case where the parent editor (Claude Desktop, Cursor, etc.) goes away without killing its MCP children -- the existing parentDeath watcher only notices when the parent reparents to PID 1, not when the parent stays alive but stops talking. With --idle-timeout=10m the server arms a timer that resets on every incoming MCP request via mcp-go's BeforeAny hook; if it fires the server runs the same cleanup path as SIGTERM. Default 0 keeps existing behavior (no timeout) so CI and interactive sessions are unaffected. Combined with the sync.Once cleanup guard, the idle path is just a third trigger of the same teardown -- not a new shutdown sequence. Adapted from upstream PR isaacphi#120 (isham703). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
judggernaut
added a commit
to judggernaut/mcp-language-server
that referenced
this pull request
Jul 3, 2026
… bug Implements the one genuinely useful idea from upstream PR isaacphi#120 (idle auto-shutdown), written fresh against this fork's main.go rather than merging that PR's broader rewrite, which conflicts with commits already in this fork (the stdin write-race fix, cleanupOnce, telemetry hooks). Add --idle-timeout / MCP_IDLE_TIMEOUT (duration, 0 = disabled). When set, a monitor goroutine shuts the server down after that long with no MCP request (tracked via a new AddBeforeAny hook alongside the existing telemetry hooks, so any request counts as activity, not just tool calls). Useful now that a single machine can end up running several persistent per-language MCP servers: idle ones can free their LSP process and file watchers instead of sitting around indefinitely. Moved the "done" channel from a local variable in main() onto the mcpServer struct so background goroutines (the idle monitor, and the pre-existing parent-death detector) can trigger the same cleanup path without main() passing it around. While wiring this up, found and fixed a real, latent bug shared by the idle-timeout path and the pre-existing parent-death detector (the "Claude desktop does not properly kill child processes" handling): both called cleanup() from a background goroutine and expected the main goroutine's blocking server.start() call to then return on its own so the process could exit. That only ever worked for OS signals, because mcp-go's own ServeStdio independently catches SIGINT/SIGTERM itself and cancels its internal context to unblock its stdio read loop; nothing does that for parent-death or idle-timeout. Confirmed empirically (minimal repro) that closing os.Stdin from another goroutine does not reliably unblock an in-flight blocking Read() on the same file descriptor on this platform, so that fix (also present in PR isaacphi#120) isn't sufficient on its own either. Both paths now call os.Exit() directly once cleanup finishes, so an orphaned or idle server actually terminates instead of hanging forever consuming resources, which is the entire point of both mechanisms. Verified with a live smoke test: process self-exits at the configured idle-timeout, and a synthetic parent-death scenario (killing an intermediate launcher process) confirms the reparent-to-PID-1 path now also terminates correctly, where it previously would have hung. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
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.
Summary
This PR addresses Swift MCP memory/process buildup observed in large workspaces by adding opt-in runtime controls and hardening shutdown behavior, while preserving existing defaults.
Changes
--watcher-preopen-on-register(defaulttrue)--watcher-preopen-max-files(default0= unlimited)--idle-timeout(default0s= disabled)PreopenOnRegistration boolPreopenMaxFiles intPreopenOnRegistration.PreopenMaxFiles(counts successful opens only).BeforeAnyhook.sync.Once) and ensure cleanup always runs when stdio loop exits.Client.Close()idempotent in LSP client to avoid repeated wait/close races.Why
In practice, duplicated MCP stacks plus registration-triggered preopen scans can create high memory pressure and process accumulation. This change gives clients an opt-out/cap for preopen and adds reliable idle/process cleanup.
Compatibility
Defaults preserve current behavior globally, so existing clients are unaffected unless they opt in.
Tests
Files
main.gointernal/watcher/interfaces.gointernal/watcher/watcher.gointernal/lsp/client.gointernal/lsp/client_close_test.gointernal/watcher/testing/mock_client.gointernal/watcher/testing/watcher_test.goREADME.md