diff --git a/.github/workflows/cgo.yml b/.github/workflows/cgo.yml index 9118ca46b17..8c066043fda 100644 --- a/.github/workflows/cgo.yml +++ b/.github/workflows/cgo.yml @@ -1038,7 +1038,7 @@ jobs: # blocking on unrelated legacy custom analyzer findings in tests or other analyzer # families. - name: Run custom linters - run: make golint-custom LINTER_FLAGS="-errstringmatch -panicinlibrarycode -test=false" + run: make golint-custom LINTER_FLAGS="-errstringmatch -panicinlibrarycode -manualmutexunlock -test=false" actions-build: runs-on: ubuntu-latest diff --git a/pkg/agentdrain/coordinator.go b/pkg/agentdrain/coordinator.go index bc063898375..0697875038f 100644 --- a/pkg/agentdrain/coordinator.go +++ b/pkg/agentdrain/coordinator.go @@ -113,8 +113,8 @@ func (c *Coordinator) LoadSnapshots(snapshots map[string][]byte) error { // minerFor retrieves the miner for the given stage, returning an error if missing. func (c *Coordinator) minerFor(stage string) (*Miner, error) { c.mu.RLock() + defer c.mu.RUnlock() m, ok := c.miners[stage] - c.mu.RUnlock() if !ok { return nil, fmt.Errorf("agentdrain: no miner registered for stage %q", stage) } diff --git a/pkg/agentdrain/miner.go b/pkg/agentdrain/miner.go index 6983678facb..ad4fb9238d6 100644 --- a/pkg/agentdrain/miner.go +++ b/pkg/agentdrain/miner.go @@ -116,10 +116,10 @@ func (m *Miner) TrainEvent(evt AgentEvent) (*MatchResult, error) { result.Stage = evt.Stage // Propagate stage to cluster. m.mu.Lock() + defer m.mu.Unlock() if c, ok := m.store.get(result.ClusterID); ok && c.Stage == "" { c.Stage = evt.Stage } - m.mu.Unlock() return result, nil } @@ -134,9 +134,12 @@ func (m *Miner) AnalyzeEvent(evt AgentEvent) (*MatchResult, *AnomalyReport, erro return nil, nil, errors.New("agentdrain: AnalyzeEvent: empty event after masking") } - m.mu.RLock() - inferResult, _ := m.findBestMatchingCluster(tokens) - m.mu.RUnlock() + inferResult := func() *MatchResult { + m.mu.RLock() + defer m.mu.RUnlock() + result, _ := m.findBestMatchingCluster(tokens) + return result + }() isNew := inferResult == nil result, err := m.TrainEvent(evt) @@ -144,10 +147,12 @@ func (m *Miner) AnalyzeEvent(evt AgentEvent) (*MatchResult, *AnomalyReport, erro return nil, nil, err } - var cluster *Cluster - m.mu.RLock() - cluster, _ = m.store.get(result.ClusterID) - m.mu.RUnlock() + cluster := func() *Cluster { + m.mu.RLock() + defer m.mu.RUnlock() + c, _ := m.store.get(result.ClusterID) + return c + }() detector, err := NewAnomalyDetector(m.cfg.SimThreshold, m.cfg.RareClusterThreshold) if err != nil { diff --git a/pkg/cli/compile_watch.go b/pkg/cli/compile_watch.go index c96243eefa6..6f85b782a93 100644 --- a/pkg/cli/compile_watch.go +++ b/pkg/cli/compile_watch.go @@ -186,27 +186,32 @@ func watchAndCompileWorkflows(ctx context.Context, markdownFile string, compiler depGraph.RemoveWorkflow(event.Name) case event.Has(fsnotify.Write) || event.Has(fsnotify.Create): // Handle file modification or creation - add to debounced compilation - debounceMu.Lock() - modifiedFiles[event.Name] = struct{}{} - - // Reset debounce timer - if debounceTimer != nil { - debounceTimer.Stop() - } - debounceTimer = time.AfterFunc(debounceDelay, func() { + func() { debounceMu.Lock() - filesToCompile := make([]string, 0, len(modifiedFiles)) - for file := range modifiedFiles { - filesToCompile = append(filesToCompile, file) + defer debounceMu.Unlock() + modifiedFiles[event.Name] = struct{}{} + + // Reset debounce timer + if debounceTimer != nil { + debounceTimer.Stop() } - // Clear the modifiedFiles map - modifiedFiles = make(map[string]struct{}) - debounceMu.Unlock() - - // Compile the modified files using dependency graph - compileModifiedFilesWithDependencies(ctx, compiler, depGraph, filesToCompile, verbose) - }) - debounceMu.Unlock() + debounceTimer = time.AfterFunc(debounceDelay, func() { + filesToCompile := func() []string { + debounceMu.Lock() + defer debounceMu.Unlock() + files := make([]string, 0, len(modifiedFiles)) + for file := range modifiedFiles { + files = append(files, file) + } + // Clear the modifiedFiles map + modifiedFiles = make(map[string]struct{}) + return files + }() + + // Compile the modified files using dependency graph + compileModifiedFilesWithDependencies(ctx, compiler, depGraph, filesToCompile, verbose) + }) + }() } case err, ok := <-watcher.Errors: diff --git a/pkg/cli/docker_images.go b/pkg/cli/docker_images.go index 22f6aba279f..9bade06af1f 100644 --- a/pkg/cli/docker_images.go +++ b/pkg/cli/docker_images.go @@ -101,14 +101,18 @@ func IsDockerImageDownloading(image string) bool { func IsDockerAvailable(ctx context.Context) bool { ctx = normalizeDockerContext(ctx) - pullState.mu.RLock() - if pullState.mockAvailableInUse { - available := pullState.mockDockerAvailable - pullState.mu.RUnlock() - dockerImagesLog.Printf("Mock: Docker available: %v", available) - return available + mockEnabled, mockAvailable := func() (bool, bool) { + pullState.mu.RLock() + defer pullState.mu.RUnlock() + if pullState.mockAvailableInUse { + return true, pullState.mockDockerAvailable + } + return false, false + }() + if mockEnabled { + dockerImagesLog.Printf("Mock: Docker available: %v", mockAvailable) + return mockAvailable } - pullState.mu.RUnlock() cmd := exec.CommandContext(ctx, "docker", "info") cmd.Stdout = nil @@ -146,9 +150,11 @@ func StartDockerImageDownload(ctx context.Context, image string) bool { // Start the download in a goroutine with retry logic go func() { defer func() { - pullState.mu.Lock() - delete(pullState.downloading, image) - pullState.mu.Unlock() + func() { + pullState.mu.Lock() + defer pullState.mu.Unlock() + delete(pullState.downloading, image) + }() if r := recover(); r != nil { dockerImagesLog.Printf("Panic in docker image download for %s (recovered): %v", image, r) } diff --git a/pkg/console/spinner.go b/pkg/console/spinner.go index 1d8d6adaf2b..bdc55058b9d 100644 --- a/pkg/console/spinner.go +++ b/pkg/console/spinner.go @@ -127,22 +127,27 @@ func NewSpinner(message string) *SpinnerWrapper { func (s *SpinnerWrapper) Start() { if s.enabled && s.program != nil { - s.mu.Lock() - if s.running { - s.mu.Unlock() + shouldStart := func() bool { + s.mu.Lock() + defer s.mu.Unlock() + if s.running { + return false + } + s.running = true + s.wg.Add(1) + return true + }() + if !shouldStart { spinnerLog.Print("Spinner already running, skipping Start") return } - s.running = true - s.wg.Add(1) - s.mu.Unlock() spinnerLog.Print("Starting spinner") go func() { defer s.wg.Done() defer func() { s.mu.Lock() + defer s.mu.Unlock() s.running = false - s.mu.Unlock() }() defer func() { if r := recover(); r != nil { @@ -156,31 +161,40 @@ func (s *SpinnerWrapper) Start() { func (s *SpinnerWrapper) Stop() { if s.enabled && s.program != nil { - s.mu.Lock() - if s.running { + wasRunning := func() bool { + s.mu.Lock() + defer s.mu.Unlock() + if !s.running { + return false + } s.running = false - s.mu.Unlock() + return true + }() + if wasRunning { spinnerLog.Print("Stopping spinner") s.program.Quit() s.wg.Wait() // Wait for the goroutine to complete fmt.Fprintf(os.Stderr, "%s%s", ansiCarriageReturn, ansiClearLine) - } else { - s.mu.Unlock() } } } func (s *SpinnerWrapper) StopWithMessage(msg string) { if s.enabled && s.program != nil { - s.mu.Lock() - if s.running { + wasRunning := func() bool { + s.mu.Lock() + defer s.mu.Unlock() + if !s.running { + return false + } s.running = false - s.mu.Unlock() + return true + }() + if wasRunning { s.program.Quit() s.wg.Wait() // Wait for the goroutine to complete fmt.Fprintf(os.Stderr, "%s%s%s\n", ansiCarriageReturn, ansiClearLine, msg) } else { - s.mu.Unlock() // Still print the message even if spinner wasn't running fmt.Fprintf(os.Stderr, "%s\n", msg) } @@ -192,9 +206,11 @@ func (s *SpinnerWrapper) StopWithMessage(msg string) { func (s *SpinnerWrapper) UpdateMessage(message string) { if s.enabled && s.program != nil { - s.mu.Lock() - running := s.running - s.mu.Unlock() + running := func() bool { + s.mu.Lock() + defer s.mu.Unlock() + return s.running + }() if running { s.program.Send(updateMessageMsg(message)) } diff --git a/pkg/parser/virtual_fs.go b/pkg/parser/virtual_fs.go index 5b9472ce67e..99fe8926809 100644 --- a/pkg/parser/virtual_fs.go +++ b/pkg/parser/virtual_fs.go @@ -112,8 +112,8 @@ const BuiltinPathPrefix = "@builtin:" // In native builds, builtin virtual files are checked first, then os.ReadFile. var readFileFunc = func(path string) ([]byte, error) { builtinVirtualFilesMu.RLock() + defer builtinVirtualFilesMu.RUnlock() content, ok := builtinVirtualFiles[path] - builtinVirtualFilesMu.RUnlock() if ok { return content, nil } diff --git a/pkg/parser/virtual_fs_wasm.go b/pkg/parser/virtual_fs_wasm.go index 1bb3074d9e9..4a0163deb03 100644 --- a/pkg/parser/virtual_fs_wasm.go +++ b/pkg/parser/virtual_fs_wasm.go @@ -35,8 +35,8 @@ func init() { readFileFunc = func(path string) ([]byte, error) { // Check builtin virtual files first (embedded engine .md files etc.) builtinVirtualFilesMu.RLock() + defer builtinVirtualFilesMu.RUnlock() builtinContent, builtinOK := builtinVirtualFiles[path] - builtinVirtualFilesMu.RUnlock() if builtinOK { return builtinContent, nil }