Skip to content

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

@github-actions

Description

@github-actions

Summary

The manualmutexunlock analyzer is registered at cmd/linters/main.go:45 but not enforced by the CI LINTER_FLAGS positive selector. Audit (Run 20) finds 16 production sites across 6 files where mu.Lock() is followed by a non-deferred mu.Unlock() — leaking the mutex on panic or early-return:

High risk (function calls between Lock and Unlock — panic-leak path)

File Lock line Unlock line Notes
pkg/agentdrain/miner.go 118 122 findBestMatchingCluster between Lock/Unlock
pkg/agentdrain/miner.go 137 139 RLock — store map access
pkg/agentdrain/miner.go 148 150 RLock — store map access
pkg/console/spinner.go 130 132, 138 program.Quit() + wg.Wait() between
pkg/console/spinner.go 143 145 program operation
pkg/console/spinner.go 159 162, 168 conditional Unlock paths
pkg/console/spinner.go 175 178, 183 conditional Unlock paths
pkg/cli/compile_watch.go 189 209 time.AfterFunc callback between
pkg/cli/compile_watch.go 197 204 callback body
pkg/cli/docker_images.go 104 107, 111 conditional Unlock branches
pkg/cli/docker_images.go 149 151 pullImage call in between

Low risk / defensible (trivial Lock-read-Unlock)

File Lock Unlock Notes
pkg/agentdrain/coordinator.go 115 117 NEW R20 — RLock + map lookup + RUnlock
pkg/console/spinner.go 195 197 trivial
pkg/parser/virtual_fs.go 102 104 trivial RLock+map lookup
pkg/parser/virtual_fs_wasm.go 37 39 NEW R20 — wasm build-tagged, mirrors virtual_fs.go

Why this matters: A panic between Lock() and Unlock() permanently deadlocks subsequent callers. The idiomatic Go pattern is defer mu.Unlock() immediately after Lock(). The codebase already follows this in 85%+ of mutex sites; these 16 are outliers.

Recommended Fix

Three options per site, in order of preference:

  1. Deferred wrapper (preferred for high-risk sites) — wrap the locked region in a closure:
    func() {
        mu.Lock()
        defer mu.Unlock()
        // ...work that may panic...
    }()
  2. Refine linter exemption — add a documented exemption (analogous to R19's panic-in-library-code exemptions for sync.Once.Do, BUG:, init(), doc-comments) for trivial Lock-read-Unlock with no calls between
  3. //nolint:manualmutexunlock with justification, only when (1) and (2) are inapplicable

Then append -manualmutexunlock to the LINTER_FLAGS selector at .github/workflows/cgo.yml:1041.

Validation

  • All 11 high-risk sites refactored to deferred wrapper or equivalent
  • Linter exemption documented in pkg/linters/manualmutexunlock/manualmutexunlock.go for the 5 trivial Lock-read-Unlock sites (if option 2 chosen)
  • make golint-custom LINTER_FLAGS="-manualmutexunlock -test=false" passes
  • Add -manualmutexunlock to CI LINTER_FLAGS
  • Existing tests still pass

Severity

Medium — No active deadlock observed in CI, but panic-resilience is a discipline worth enforcing. Multiple high-risk sites have non-trivial function calls between Lock and Unlock (program.Quit, time.AfterFunc, pullImage).

Sergo Context

  • Tracked since R19 (2026-05-26). Previously filed and auto-expired (create-issue.expires: 7d).
  • Re-filing Run 20 with updated count: 13 → 16 sites (new: agentdrain/coordinator.go:115, parser/virtual_fs_wasm.go:37).
  • Related linter R19: panic-in-library-code enforced after similar exemption refinement.
  • See #aw_sg20a1 for the zero-violation enforcement upgrade (osexitinlibrary + rawloginlib).

Filed by Sergo Run 20 — workflow run §26491943719

Generated by 🤖 Sergo - Serena Go Expert · opus47 16.9M ·

  • expires on Jun 3, 2026, 5:16 AM UTC

Metadata

Metadata

Labels

cookieIssue Monster Loves Cookies!sergo

Type

No type
No fields configured for issues without a type.

Projects

No projects

Milestone

No milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions