d66dfa2 · crush · git.iko.soy

git.iko.soy/crush

fix(lsp): use csync for lsp clients (#1073)

The map was being passed down everywhere, but the locking mechanism only ever lived in `app.go`, which might cause concurrent access issues. This changes it to a `*csync.Map`. Signed-off-by: Carlos Alexandro Becker <caarlos0@users.noreply.github.com>

commitd66dfa2c92173e85d707ea20edceed7b9aba32d4 authorCarlos Alexandro Becker <caarlos0@users.noreply.github.com> dateThu 18 Sep 2025, 11:16 (1 year ago) parent016b3b6

13 files changed+64−53

compare with main

unifiedsplit

internal/app/app.go+4−12 internal/app/lsp.go+1−3 internal/csync/maps.go+4−4 internal/csync/versionedmap.go+20−4 internal/llm/agent/agent.go+3−4 internal/llm/tools/diagnostics.go+10−10 internal/llm/tools/edit.go+3−2 internal/llm/tools/multiedit.go+3−2 internal/llm/tools/view.go+3−2 internal/llm/tools/write.go+3−2 internal/tui/components/chat/header/header.go+4−3 internal/tui/components/chat/sidebar/sidebar.go+2−2 internal/tui/components/lsp/lsp.go+4−3

internal/app/app.go+4−12

@@ -6,12 +6,12 @@ import (

66 "errors"

77 "fmt"

88 "log/slog"

9− "maps"

109 "sync"

1110 "time"

1211

1312 tea "github.com/charmbracelet/bubbletea/v2"

1413 "github.com/charmbracelet/crush/internal/config"

14+ "github.com/charmbracelet/crush/internal/csync"

1515 "github.com/charmbracelet/crush/internal/db"

1616 "github.com/charmbracelet/crush/internal/format"

1717 "github.com/charmbracelet/crush/internal/history"

@@ -33,9 +33,7 @@ type App struct {

3333

3434 CoderAgent agent.Service

3535

36− LSPClients map[string]*lsp.Client

37−

38− clientsMutex sync.RWMutex

36+ LSPClients *csync.Map[string, *lsp.Client]

3937

4038 config *config.Config

4139

@@ -66,7 +64,7 @@ func New(ctx context.Context, conn *sql.DB, cfg *config.Config) (*App, error) {

6664 Messages: messages,

6765 History: files,

6866 Permissions: permission.NewPermissionService(cfg.WorkingDir(), skipPermissionsRequests, allowedTools),

69− LSPClients: make(map[string]*lsp.Client),

67+ LSPClients: csync.NewMap[string, *lsp.Client](),

7068

7169 globalCtx: ctx,

7270

@@ -324,14 +322,8 @@ func (app *App) Shutdown() {

324322 app.CoderAgent.CancelAll()

325323 }

326324

327− // Get all LSP clients.

328− app.clientsMutex.RLock()

329− clients := make(map[string]*lsp.Client, len(app.LSPClients))

330− maps.Copy(clients, app.LSPClients)

331− app.clientsMutex.RUnlock()

332−

333325 // Shutdown all LSP clients.

334− for name, client := range clients {

326+ for name, client := range app.LSPClients.Seq2() {

335327 shutdownCtx, cancel := context.WithTimeout(app.globalCtx, 5*time.Second)

336328 if err := client.Close(shutdownCtx); err != nil {

337329 slog.Error("Failed to shutdown LSP client", "name", name, "error", err)

internal/app/lsp.go+1−3

@@ -76,7 +76,5 @@ func (app *App) createAndStartLSPClient(ctx context.Context, name string, config

7676 slog.Info("LSP client initialized", "name", name)

7777

7878 // Add to map with mutex protection before starting goroutine

79− app.clientsMutex.Lock()

80− app.LSPClients[name] = lspClient

81− app.clientsMutex.Unlock()

79+ app.LSPClients.Set(name, lspClient)

8280}

internal/csync/maps.go+4−4

@@ -70,10 +70,10 @@ func (m *Map[K, V]) GetOrSet(key K, fn func() V) V {

7070

7171// Take gets an item and then deletes it.

7272func (m *Map[K, V]) Take(key K) (V, bool) {

73− v, ok := m.Get(key)

74− if ok {

75− m.Del(key)

76− }

73+ m.mu.Lock()

74+ defer m.mu.Unlock()

75+ v, ok := m.inner[key]

76+ delete(m.inner, key)

7777 return v, ok

7878}

7979

internal/csync/versionedmap.go+20−4

@@ -1,34 +1,50 @@

11package csync

33import (

4+ "iter"

45 "sync/atomic"

56)

78// NewVersionedMap creates a new versioned, thread-safe map.

89func NewVersionedMap[K comparable, V any]() *VersionedMap[K, V] {

910 return &VersionedMap[K, V]{

10− Map: NewMap[K, V](),

11+ m: NewMap[K, V](),

1112 }

1213}

1314

1415// VersionedMap is a thread-safe map that keeps track of its version.

1516type VersionedMap[K comparable, V any] struct {

16− *Map[K, V]

17+ m *Map[K, V]

1718 v atomic.Uint64

1819}

1920

21+// Get gets the value for the specified key from the map.

22+func (m *VersionedMap[K, V]) Get(key K) (V, bool) {

23+ return m.m.Get(key)

24+}

25+

2026// Set sets the value for the specified key in the map and increments the version.

2127func (m *VersionedMap[K, V]) Set(key K, value V) {

22− m.Map.Set(key, value)

28+ m.m.Set(key, value)

2329 m.v.Add(1)

2430}

2531

2632// Del deletes the specified key from the map and increments the version.

2733func (m *VersionedMap[K, V]) Del(key K) {

28− m.Map.Del(key)

34+ m.m.Del(key)

2935 m.v.Add(1)

3036}

3137

38+// Seq2 returns an iter.Seq2 that yields key-value pairs from the map.

39+func (m *VersionedMap[K, V]) Seq2() iter.Seq2[K, V] {

40+ return m.m.Seq2()

41+}

42+

43+// Len returns the number of items in the map.

44+func (m *VersionedMap[K, V]) Len() int {

45+ return m.m.Len()

46+}

47+

3248// Version returns the current version of the map.

3349func (m *VersionedMap[K, V]) Version() uint64 {

3450 return m.v.Load()

internal/llm/agent/agent.go+3−4

@@ -83,8 +83,7 @@ type agent struct {

8383 summarizeProviderID string

8484

8585 activeRequests *csync.Map[string, context.CancelFunc]

86−

87− promptQueue *csync.Map[string, []string]

86+ promptQueue *csync.Map[string, []string]

8887}

8988

9089var agentPromptMap = map[string]prompt.PromptID{

@@ -100,7 +99,7 @@ func NewAgent(

10099 sessions session.Service,

101100 messages message.Service,

102101 history history.Service,

103− lspClients map[string]*lsp.Client,

102+ lspClients *csync.Map[string, *lsp.Client],

104103) (Service, error) {

105104 cfg := config.Get()

106105

@@ -204,7 +203,7 @@ func NewAgent(

204203 withCoderTools := func(t []tools.BaseTool) []tools.BaseTool {

205204 if agentCfg.ID == "coder" {

206205 t = append(t, mcpTools...)

207− if len(lspClients) > 0 {

206+ if lspClients.Len() > 0 {

208207 t = append(t, tools.NewDiagnosticsTool(lspClients))

209208 }

210209 }

internal/llm/tools/diagnostics.go+10−10

@@ -9,6 +9,7 @@ import (

99 "strings"

1010 "time"

1111

12+ "github.com/charmbracelet/crush/internal/csync"

1213 "github.com/charmbracelet/crush/internal/lsp"

1314 "github.com/charmbracelet/x/powernap/pkg/lsp/protocol"

1415)

@@ -18,7 +19,7 @@ type DiagnosticsParams struct {

1819}

1920

2021type diagnosticsTool struct {

21− lspClients map[string]*lsp.Client

22+ lspClients *csync.Map[string, *lsp.Client]

2223}

2324

2425const (

@@ -46,7 +47,7 @@ TIPS:

4647`

4748)

4849

49−func NewDiagnosticsTool(lspClients map[string]*lsp.Client) BaseTool {

50+func NewDiagnosticsTool(lspClients *csync.Map[string, *lsp.Client]) BaseTool {

5051 return &diagnosticsTool{

5152 lspClients,

5253 }

@@ -76,20 +77,19 @@ func (b *diagnosticsTool) Run(ctx context.Context, call ToolCall) (ToolResponse,

7677 return NewTextErrorResponse(fmt.Sprintf("error parsing parameters: %s", err)), nil

7778 }

7879

79− lsps := b.lspClients

80− if len(lsps) == 0 {

80+ if b.lspClients.Len() == 0 {

8181 return NewTextErrorResponse("no LSP clients available"), nil

8282 }

83− notifyLSPs(ctx, lsps, params.FilePath)

84− output := getDiagnostics(params.FilePath, lsps)

83+ notifyLSPs(ctx, b.lspClients, params.FilePath)

84+ output := getDiagnostics(params.FilePath, b.lspClients)

8585 return NewTextResponse(output), nil

8686}

8787

88−func notifyLSPs(ctx context.Context, lsps map[string]*lsp.Client, filepath string) {

88+func notifyLSPs(ctx context.Context, lsps *csync.Map[string, *lsp.Client], filepath string) {

8989 if filepath == "" {

9090 return

9191 }

92− for _, client := range lsps {

92+ for client := range lsps.Seq() {

9393 if !client.HandlesFile(filepath) {

9494 continue

9595 }

@@ -99,11 +99,11 @@ func notifyLSPs(ctx context.Context, lsps map[string]*lsp.Client, filepath strin

9999 }

100100}

101101

102−func getDiagnostics(filePath string, lsps map[string]*lsp.Client) string {

102+func getDiagnostics(filePath string, lsps *csync.Map[string, *lsp.Client]) string {

103103 fileDiagnostics := []string{}

104104 projectDiagnostics := []string{}

105105

106− for lspName, client := range lsps {

106+ for lspName, client := range lsps.Seq2() {

107107 for location, diags := range client.GetDiagnostics() {

108108 path, err := location.Path()

109109 if err != nil {

internal/llm/tools/edit.go+3−2

@@ -10,6 +10,7 @@ import (

1010 "strings"

1111 "time"

1212

13+ "github.com/charmbracelet/crush/internal/csync"

1314 "github.com/charmbracelet/crush/internal/diff"

1415 "github.com/charmbracelet/crush/internal/fsext"

1516 "github.com/charmbracelet/crush/internal/history"

@@ -39,7 +40,7 @@ type EditResponseMetadata struct {

3940}

4041

4142type editTool struct {

42− lspClients map[string]*lsp.Client

43+ lspClients *csync.Map[string, *lsp.Client]

4344 permissions permission.Service

4445 files history.Service

4546 workingDir string

@@ -104,7 +105,7 @@ WINDOWS NOTES:

104105Remember: when making multiple file edits in a row to the same file, you should prefer to send all edits in a single message with multiple calls to this tool, rather than multiple messages with a single call each.`

105106)

106107

107−func NewEditTool(lspClients map[string]*lsp.Client, permissions permission.Service, files history.Service, workingDir string) BaseTool {

108+func NewEditTool(lspClients *csync.Map[string, *lsp.Client], permissions permission.Service, files history.Service, workingDir string) BaseTool {

108109 return &editTool{

109110 lspClients: lspClients,

110111 permissions: permissions,

internal/llm/tools/multiedit.go+3−2

@@ -10,6 +10,7 @@ import (

1010 "strings"

1111 "time"

1212

13+ "github.com/charmbracelet/crush/internal/csync"

1314 "github.com/charmbracelet/crush/internal/diff"

1415 "github.com/charmbracelet/crush/internal/fsext"

1516 "github.com/charmbracelet/crush/internal/history"

@@ -43,7 +44,7 @@ type MultiEditResponseMetadata struct {

4344}

4445

4546type multiEditTool struct {

46− lspClients map[string]*lsp.Client

47+ lspClients *csync.Map[string, *lsp.Client]

4748 permissions permission.Service

4849 files history.Service

4950 workingDir string

@@ -95,7 +96,7 @@ If you want to create a new file, use:

9596- Subsequent edits: normal edit operations on the created content`

9697)

9798

98−func NewMultiEditTool(lspClients map[string]*lsp.Client, permissions permission.Service, files history.Service, workingDir string) BaseTool {

99+func NewMultiEditTool(lspClients *csync.Map[string, *lsp.Client], permissions permission.Service, files history.Service, workingDir string) BaseTool {

99100 return &multiEditTool{

100101 lspClients: lspClients,

101102 permissions: permissions,

internal/llm/tools/view.go+3−2

@@ -11,6 +11,7 @@ import (

1111 "strings"

1212 "unicode/utf8"

1313

14+ "github.com/charmbracelet/crush/internal/csync"

1415 "github.com/charmbracelet/crush/internal/lsp"

1516 "github.com/charmbracelet/crush/internal/permission"

1617)

@@ -28,7 +29,7 @@ type ViewPermissionsParams struct {

2829}

2930

3031type viewTool struct {

31− lspClients map[string]*lsp.Client

32+ lspClients *csync.Map[string, *lsp.Client]

3233 workingDir string

3334 permissions permission.Service

3435}

@@ -81,7 +82,7 @@ TIPS:

8182- When viewing large files, use the offset parameter to read specific sections`

8283)

8384

84−func NewViewTool(lspClients map[string]*lsp.Client, permissions permission.Service, workingDir string) BaseTool {

85+func NewViewTool(lspClients *csync.Map[string, *lsp.Client], permissions permission.Service, workingDir string) BaseTool {

8586 return &viewTool{

8687 lspClients: lspClients,

8788 workingDir: workingDir,

internal/llm/tools/write.go+3−2

@@ -10,6 +10,7 @@ import (

1010 "strings"

1111 "time"

1212

13+ "github.com/charmbracelet/crush/internal/csync"

1314 "github.com/charmbracelet/crush/internal/diff"

1415 "github.com/charmbracelet/crush/internal/fsext"

1516 "github.com/charmbracelet/crush/internal/history"

@@ -30,7 +31,7 @@ type WritePermissionsParams struct {

3031}

3132

3233type writeTool struct {

33− lspClients map[string]*lsp.Client

34+ lspClients *csync.Map[string, *lsp.Client]

3435 permissions permission.Service

3536 files history.Service

3637 workingDir string

@@ -78,7 +79,7 @@ TIPS:

7879- Always include descriptive comments when making changes to existing code`

7980)

8081

81−func NewWriteTool(lspClients map[string]*lsp.Client, permissions permission.Service, files history.Service, workingDir string) BaseTool {

82+func NewWriteTool(lspClients *csync.Map[string, *lsp.Client], permissions permission.Service, files history.Service, workingDir string) BaseTool {

8283 return &writeTool{

8384 lspClients: lspClients,

8485 permissions: permissions,

internal/tui/components/chat/header/header.go+4−3

@@ -6,6 +6,7 @@ import (

77 tea "github.com/charmbracelet/bubbletea/v2"

88 "github.com/charmbracelet/crush/internal/config"

9+ "github.com/charmbracelet/crush/internal/csync"

910 "github.com/charmbracelet/crush/internal/fsext"

1011 "github.com/charmbracelet/crush/internal/lsp"

1112 "github.com/charmbracelet/crush/internal/pubsub"

@@ -28,11 +29,11 @@ type Header interface {

2829type header struct {

2930 width int

3031 session session.Session

31− lspClients map[string]*lsp.Client

32+ lspClients *csync.Map[string, *lsp.Client]

3233 detailsOpen bool

3334}

3435

35−func New(lspClients map[string]*lsp.Client) Header {

36+func New(lspClients *csync.Map[string, *lsp.Client]) Header {

3637 return &header{

3738 lspClients: lspClients,

3839 width: 0,

@@ -104,7 +105,7 @@ func (h *header) details(availWidth int) string {

104105 var parts []string

105106

106107 errorCount := 0

107− for _, l := range h.lspClients {

108+ for l := range h.lspClients.Seq() {

108109 for _, diagnostics := range l.GetDiagnostics() {

109110 for _, diagnostic := range diagnostics {

110111 if diagnostic.Severity == protocol.SeverityError {

internal/tui/components/chat/sidebar/sidebar.go+2−2

@@ -69,13 +69,13 @@ type sidebarCmp struct {

6969 session session.Session

7070 logo string

7171 cwd string

72− lspClients map[string]*lsp.Client

72+ lspClients *csync.Map[string, *lsp.Client]

7373 compactMode bool

7474 history history.Service

7575 files *csync.Map[string, SessionFile]

7676}

7777

78−func New(history history.Service, lspClients map[string]*lsp.Client, compact bool) Sidebar {

78+func New(history history.Service, lspClients *csync.Map[string, *lsp.Client], compact bool) Sidebar {

7979 return &sidebarCmp{

8080 lspClients: lspClients,

8181 history: history,

internal/tui/components/lsp/lsp.go+4−3

@@ -6,6 +6,7 @@ import (

77 "github.com/charmbracelet/crush/internal/app"

88 "github.com/charmbracelet/crush/internal/config"

9+ "github.com/charmbracelet/crush/internal/csync"

910 "github.com/charmbracelet/crush/internal/lsp"

1011 "github.com/charmbracelet/crush/internal/tui/components/core"

1112 "github.com/charmbracelet/crush/internal/tui/styles"

@@ -22,7 +23,7 @@ type RenderOptions struct {

2223}

2324

2425// RenderLSPList renders a list of LSP status items with the given options.

25−func RenderLSPList(lspClients map[string]*lsp.Client, opts RenderOptions) []string {

26+func RenderLSPList(lspClients *csync.Map[string, *lsp.Client], opts RenderOptions) []string {

2627 t := styles.CurrentTheme()

2728 lspList := []string{}

2829

@@ -91,7 +92,7 @@ func RenderLSPList(lspClients map[string]*lsp.Client, opts RenderOptions) []stri

9192 protocol.SeverityHint: 0,

9293 protocol.SeverityInformation: 0,

9394 }

94− if client, ok := lspClients[l.Name]; ok {

95+ if client, ok := lspClients.Get(l.Name); ok {

9596 for _, diagnostics := range client.GetDiagnostics() {

9697 for _, diagnostic := range diagnostics {

9798 if severity, ok := lspErrs[diagnostic.Severity]; ok {

@@ -134,7 +135,7 @@ func RenderLSPList(lspClients map[string]*lsp.Client, opts RenderOptions) []stri

134135}

135136

136137// RenderLSPBlock renders a complete LSP block with optional truncation indicator.

137−func RenderLSPBlock(lspClients map[string]*lsp.Client, opts RenderOptions, showTruncationIndicator bool) string {

138+func RenderLSPBlock(lspClients *csync.Map[string, *lsp.Client], opts RenderOptions, showTruncationIndicator bool) string {

138139 t := styles.CurrentTheme()

139140 lspList := RenderLSPList(lspClients, opts)

140141