Skip to content

refactor(lsp): coalesce concurrent server startup in manager#2493

Open
iceymoss wants to merge 1 commit intocharmbracelet:mainfrom
iceymoss:refactor/lsp-singleflight-startup
Open

refactor(lsp): coalesce concurrent server startup in manager#2493
iceymoss wants to merge 1 commit intocharmbracelet:mainfrom
iceymoss:refactor/lsp-singleflight-startup

Conversation

@iceymoss
Copy link
Copy Markdown
Contributor

Summary

Refactor internal/lsp/manager.go to coalesce concurrent LSP startup requests per server name using singleflight.

Previously, startServer() relied on repeated check-then-create paths. Under concurrent calls, multiple goroutines could still race to create duplicate clients, then close losers. This worked functionally but added unnecessary startup churn and complexity.

Changes

  • Add singleflight.Group to lsp.Manager:
    • requestGroup singleflight.Group
  • Refactor startServer() into a clearer flow:
    • fast-path checks (unavailable, existing active client),
    • preflight checks (LookPath, skip list, handles),
    • singleflight-protected startup critical section.
  • Add helper functions for active client checks:
    • isActiveClient(*Client) bool
    • getActiveClient(name string) (*Client, bool)
  • Inside the startup critical section:
    • double-check active client before creating,
    • create/register client once,
    • initialize with timeout,
    • set terminal state (StateReady / StateError),
    • remove failed client from s.clients.
  • Use context.WithoutCancel(ctx) for shared startup work so one waiter cancellation does not abort the shared startup operation.
  • Reduce duplicate error logs for shared singleflight waiters.

Why

  • Avoid redundant client creation under concurrent Start() calls.
  • Reduce process churn and improve startup determinism.
  • Keep callback/state propagation semantics for app/UI unchanged.

Compatibility

  • No public API changes.
  • Manager.Start usage remains unchanged.
  • Existing server state model and callback behavior are preserved.

Verification

  • go test ./internal/lsp
  • go test ./internal/lsp/...

Notes

Waiter cancellation now only stops waiting for that caller; the shared startup operation may continue for other callers.

@iceymoss iceymoss requested a review from a team as a code owner March 26, 2026 11:00
@iceymoss iceymoss requested review from meowgorithm and raphamorim and removed request for a team March 26, 2026 11:00
@charmcli
Copy link
Copy Markdown
Contributor

charmcli commented Mar 26, 2026

All contributors have signed the CLA ✍️ ✅
Posted by the CLA Assistant Lite bot.

@iceymoss
Copy link
Copy Markdown
Contributor Author

I have read the Contributor License Agreement (CLA) and hereby sign the CLA.

@iceymoss
Copy link
Copy Markdown
Contributor Author

recheck

Use singleflight in LSP manager to deduplicate concurrent startup requests for the same server, keep callback/state semantics intact, and clean up failed initialization paths.

Made-with: Cursor
@iceymoss iceymoss force-pushed the refactor/lsp-singleflight-startup branch from 9a265b3 to 0c745ef Compare March 26, 2026 13:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants