From 6f1c8bf7a36eb7044ed7aad30f84664cbbf0d303 Mon Sep 17 00:00:00 2001 From: Paul Buetow Date: Tue, 17 Mar 2026 11:28:19 +0200 Subject: Fix bugs, remove duplication, and clean up code quality issues - Log swallowed JSON unmarshal errors in stats and LSP handlers - Fix debug log file handle leak in tmuxedit (return closer from initDebugLog) - Check f.Close() errors on write paths in promptstore and tmuxedit - Fix cacheGet TOCTOU race by using single write lock - Fix readInput to use passed stdin reader instead of os.Stdin.Stat() - Remove 45 'moved to' comment tombstones from lsp/handlers.go - Deduplicate canonicalProvider wrappers (use llmutils.CanonicalProvider directly) - Remove SetWindow side effect from stats.TakeSnapshot (pure read now) - Move duplicated splitLines to textutil.SplitLinesBytes - Collapse StatusSink.SetGlobal 10 params into GlobalStatus struct - Simplify LRU touchLocked to in-place delete-and-append Co-Authored-By: Claude Opus 4.6 (1M context) --- internal/lsp/completion_state.go | 20 ++++++++------------ 1 file changed, 8 insertions(+), 12 deletions(-) (limited to 'internal/lsp/completion_state.go') diff --git a/internal/lsp/completion_state.go b/internal/lsp/completion_state.go index 692eafe..5c2716f 100644 --- a/internal/lsp/completion_state.go +++ b/internal/lsp/completion_state.go @@ -69,18 +69,16 @@ func (s *completionState) takePendingCompletion(key string) []CompletionItem { return cpy } -// cacheGet returns the cached value for key. A read lock is sufficient for -// cache misses. On a hit we must promote to a write lock so touchLocked can -// update the LRU order. +// cacheGet returns the cached value for key. Uses a single write lock to +// avoid a TOCTOU race between the lookup and the LRU touch — the key could +// be evicted between an RUnlock and a subsequent Lock promotion. func (s *completionState) cacheGet(key string) (string, bool) { - s.stateMu.RLock() + s.stateMu.Lock() + defer s.stateMu.Unlock() v, ok := s.compCache[key] - s.stateMu.RUnlock() if !ok { return "", false } - s.stateMu.Lock() - defer s.stateMu.Unlock() s.touchLocked(key) return v, true } @@ -105,17 +103,15 @@ func (s *completionState) cachePut(key, value string) { s.touchLocked(key) } +// touchLocked moves key to the end of the LRU order list. +// Uses delete-and-append: remove the existing entry in-place, then append. func (s *completionState) touchLocked(key string) { - idx := -1 for i, k := range s.compCacheOrder { if k == key { - idx = i + s.compCacheOrder = append(s.compCacheOrder[:i], s.compCacheOrder[i+1:]...) break } } - if idx >= 0 { - s.compCacheOrder = append(append([]string{}, s.compCacheOrder[:idx]...), s.compCacheOrder[idx+1:]...) - } s.compCacheOrder = append(s.compCacheOrder, key) } -- cgit v1.2.3