Memory leak: LSP client diagnostics and files maps not cleared on shutdown #6632

Closed
opened 2026-02-16 18:04:48 -05:00 by yindo · 3 comments
Owner

Originally created by @sauerdaniel on GitHub (Jan 17, 2026).

Originally assigned to: @rekram1-node on GitHub.

Problem

When an LSP client shuts down, the diagnostics Map and files object are not cleared in the shutdown() method of LSPClient. This can cause:

  1. Memory retention - Diagnostics data for closed files remains in memory
  2. Stale data - If the same LSP server is reconnected, old diagnostics may persist
  3. Memory growth - Over multiple LSP reconnections, maps accumulate entries

Code Location

packages/opencode/src/lsp/client.ts - The shutdown() method closes the connection but doesn't clear internal state.

Proposed Solution

Add cleanup calls in the shutdown() method:

diagnostics.clear()
for (const path in files) delete files[path]

Impact

  • Minimal change (2 lines)
  • Ensures clean state on LSP shutdown
  • Prevents memory growth from accumulated diagnostics

Relates to #5363

Originally created by @sauerdaniel on GitHub (Jan 17, 2026). Originally assigned to: @rekram1-node on GitHub. ## Problem When an LSP client shuts down, the `diagnostics` Map and `files` object are not cleared in the `shutdown()` method of `LSPClient`. This can cause: 1. **Memory retention** - Diagnostics data for closed files remains in memory 2. **Stale data** - If the same LSP server is reconnected, old diagnostics may persist 3. **Memory growth** - Over multiple LSP reconnections, maps accumulate entries ### Code Location `packages/opencode/src/lsp/client.ts` - The `shutdown()` method closes the connection but doesn't clear internal state. ## Proposed Solution Add cleanup calls in the `shutdown()` method: ```typescript diagnostics.clear() for (const path in files) delete files[path] ``` ## Impact - Minimal change (2 lines) - Ensures clean state on LSP shutdown - Prevents memory growth from accumulated diagnostics Relates to #5363
yindo added the perf label 2026-02-16 18:04:48 -05:00
yindo closed this issue 2026-02-16 18:04:48 -05:00
Author
Owner

@github-actions[bot] commented on GitHub (Jan 17, 2026):

This issue might be a duplicate of or closely related to existing memory management issues. Please check:

  • #9140: Unbounded caches cause memory growth over time
  • #5363: opencode eating 70gb of memory?
  • #7261: v1.1.6 still has memory bloat - heap not released + MCP orphan processes
  • #8947: [FEATURE]: TUI whole and LSP memory limits as part of configuration

These issues all address memory retention and cleanup problems in OpenCode, including LSP-related memory leaks. Your proposed fix for clearing diagnostics and files maps in LSPClient.shutdown() is directly addressing part of the root cause identified in #9140 regarding unbounded caches.

Feel free to ignore if your specific case differs from these.

@github-actions[bot] commented on GitHub (Jan 17, 2026): This issue might be a duplicate of or closely related to existing memory management issues. Please check: - #9140: Unbounded caches cause memory growth over time - #5363: opencode eating 70gb of memory? - #7261: v1.1.6 still has memory bloat - heap not released + MCP orphan processes - #8947: [FEATURE]: TUI whole and LSP memory limits as part of configuration These issues all address memory retention and cleanup problems in OpenCode, including LSP-related memory leaks. Your proposed fix for clearing diagnostics and files maps in LSPClient.shutdown() is directly addressing part of the root cause identified in #9140 regarding unbounded caches. Feel free to ignore if your specific case differs from these.
Author
Owner

@aptor9 commented on GitHub (Jan 18, 2026):

This is what I got from claude. Maybe worth a double check?

Do I need garbage collect diagnostics or paths variables in lsp/client.ts?

● Looking at this code, there are two data structures that accumulate state:

  1. diagnostics (line 51) - Map<string, Diagnostic[]> - Stores diagnostics per file path
  2. files (line 135-137) - { [path: string]: number } - Tracks document versions

Short answer: No, you likely don't need to.

Here's why:

  1. Lifecycle is tied to the LSP client - Both maps are naturally garbage collected when the client calls shutdown() (line 239-245) and the client object is released.
  2. diagnostics is self-updating - When the LSP server clears diagnostics for a file, it sends an empty array via publishDiagnostics, which updates the map correctly. It's also
    cleared on file open (line 194).
  3. files versioning is needed - The version numbers must persist to correctly distinguish between didOpen vs didChange notifications. Clearing this could cause protocol issues
    if a file is re-opened (version conflicts).
  4. Memory impact is minimal - These store string keys with small values (arrays or numbers). Even with hundreds of files, the memory footprint is negligible.

When you might want to consider it:

  • If the LSP client lives for extremely long sessions with thousands of unique files being touched
  • If you implement a textDocument/didClose notification (currently missing), you could clean up both maps when a file is explicitly closed

For typical usage patterns, the current design is fine.

@aptor9 commented on GitHub (Jan 18, 2026): This is what I got from claude. Maybe worth a double check? > Do I need garbage collect diagnostics or paths variables in lsp/client.ts? ● Looking at this code, there are two data structures that accumulate state: 1. diagnostics (line 51) - Map<string, Diagnostic[]> - Stores diagnostics per file path 2. files (line 135-137) - { [path: string]: number } - Tracks document versions Short answer: No, you likely don't need to. Here's why: 1. Lifecycle is tied to the LSP client - Both maps are naturally garbage collected when the client calls shutdown() (line 239-245) and the client object is released. 2. diagnostics is self-updating - When the LSP server clears diagnostics for a file, it sends an empty array via publishDiagnostics, which updates the map correctly. It's also cleared on file open (line 194). 3. files versioning is needed - The version numbers must persist to correctly distinguish between didOpen vs didChange notifications. Clearing this could cause protocol issues if a file is re-opened (version conflicts). 4. Memory impact is minimal - These store string keys with small values (arrays or numbers). Even with hundreds of files, the memory footprint is negligible. When you might want to consider it: - If the LSP client lives for extremely long sessions with thousands of unique files being touched - If you implement a textDocument/didClose notification (currently missing), you could clean up both maps when a file is explicitly closed For typical usage patterns, the current design is fine.
Author
Owner

@sauerdaniel commented on GitHub (Jan 19, 2026):

This was already fixed in commit f936043dd (Jan 15, 2026):

diagnostics.clear()
for (const path in files) delete files[path]

The fix is exactly as proposed in this issue. See: https://github.com/anomalyco/opencode/commit/f936043dd06588dc5fe1e97aca628073c23194ef

@sauerdaniel commented on GitHub (Jan 19, 2026): This was already fixed in commit f936043dd (Jan 15, 2026): ```typescript diagnostics.clear() for (const path in files) delete files[path] ``` The fix is exactly as proposed in this issue. See: https://github.com/anomalyco/opencode/commit/f936043dd06588dc5fe1e97aca628073c23194ef
Sign in to join this conversation.
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: anomalyco/opencode#6632