Skip to content

Enforce manualmutexunlock in CI and eliminate non-deferred mutex unlock paths#35189

Merged
pelikhan merged 4 commits into
mainfrom
copilot/fix-manualmutexunlock-analyzer
May 27, 2026
Merged

Enforce manualmutexunlock in CI and eliminate non-deferred mutex unlock paths#35189
pelikhan merged 4 commits into
mainfrom
copilot/fix-manualmutexunlock-analyzer

Conversation

Copy link
Copy Markdown
Contributor

Copilot AI commented May 27, 2026

manualmutexunlock was registered but not enforced in CI, allowing production lock/unlock sequences that could leak locks on panic/early-return. This PR enables enforcement and refactors the remaining flagged sites to use deferred unlock patterns.

  • CI enforcement

    • Updated .github/workflows/cgo.yml custom linter selector to include -manualmutexunlock in LINTER_FLAGS.
  • Panic-safe lock handling in production paths

    • Replaced manual Lock/RLockUnlock/RUnlock pairs with deferred unlocks (often via small scoped closures to keep lock lifetime tight) in:
      • pkg/agentdrain/miner.go
      • pkg/agentdrain/coordinator.go
      • pkg/console/spinner.go
      • pkg/cli/compile_watch.go
      • pkg/cli/docker_images.go
      • pkg/parser/virtual_fs.go
      • pkg/parser/virtual_fs_wasm.go
  • Refactor pattern used

    • Converted branchy/manual unlock blocks into scope-contained critical sections with defer:
running := func() bool {
    s.mu.Lock()
    defer s.mu.Unlock()
    if !s.running {
        return false
    }
    s.running = false
    return true
}()

Co-authored-by: gh-aw-bot <259018956+gh-aw-bot@users.noreply.github.com>
Copilot AI changed the title [WIP] Fix manualmutexunlock analyzer not enforced by CI Enforce manualmutexunlock in CI and eliminate non-deferred mutex unlock paths May 27, 2026
Copilot AI requested a review from gh-aw-bot May 27, 2026 11:09
@pelikhan pelikhan marked this pull request as ready for review May 27, 2026 11:14
Copilot AI review requested due to automatic review settings May 27, 2026 11:14
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR enables CI enforcement of the manualmutexunlock custom linter and refactors remaining production mutex unlock paths to use deferred unlock patterns for panic-safe lock release.

Changes:

  • Adds -manualmutexunlock to the custom linter flags in the CGO workflow.
  • Converts manual mutex unlock sequences to defer-based unlocks.
  • Uses scoped closures where needed to keep critical sections narrow.
Show a summary per file
File Description
.github/workflows/cgo.yml Enables manualmutexunlock in the enforced custom linter set.
pkg/agentdrain/coordinator.go Defers coordinator read-lock release in minerFor.
pkg/agentdrain/miner.go Defers miner locks in event training and analysis paths.
pkg/cli/compile_watch.go Refactors debounce mutex handling to deferred unlocks.
pkg/cli/docker_images.go Refactors Docker mock state and download cleanup locking.
pkg/console/spinner.go Refactors spinner lifecycle locking to deferred unlocks.
pkg/parser/virtual_fs.go Defers builtin virtual file read-lock release.
pkg/parser/virtual_fs_wasm.go Defers builtin virtual file read-lock release in wasm builds.

Copilot's findings

Tip

Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

  • Files reviewed: 8/8 changed files
  • Comments generated: 0

@pelikhan pelikhan merged commit ae4a3f3 into main May 27, 2026
26 checks passed
@pelikhan pelikhan deleted the copilot/fix-manualmutexunlock-analyzer branch May 27, 2026 11:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[sergo] manualmutexunlock analyzer registered but unenforced — 16 prod Lock/Unlock pairs leak on panic (#aw_sg19a1)

4 participants