manualmutexunlock false negative: distinct struct instances sharing a mutex field collapse to one tracking key (Selections.Obj() [Content truncated due to length]

[github-actions]

Summary

manualmutexunlock (a CI-enforced linter) tracks each mutex's lock/defer/manual-unlock state in a map[types.Object]*mutexVarState. For a field selector receiver such as a.mu, the receiver object is resolved via pass.TypesInfo.Selections[r].Obj(), which returns the field declaration (guarded.mu) — a single *types.Var shared by every value of that struct type. So two distinct instances a.mu and b.mu collapse to the same map key, and their states clobber each other. The net effect is a masking false negative: a genuine non-deferred unlock silently passes CI — defeating the linter's entire purpose (catching unlocks that deadlock on panic / early return).

Location

• pkg/linters/manualmutexunlock/manualmutexunlock.go:169-183 — getMutexReceiverObj; the *ast.SelectorExpr branch returns pass.TypesInfo.Selections[r].Obj() (the field, not the receiver instance)
• pkg/linters/manualmutexunlock/manualmutexunlock.go:58 — mutexVars := make(map[types.Object]*mutexVarState) keyed by that object
• pkg/linters/manualmutexunlock/manualmutexunlock.go:109-111 — second Lock() on the same key overwrites the prior instance's state

Reproduction (uses the existing guarded testdata type)

type guarded struct{ mu sync.Mutex }

func twoGuards(a, b *guarded) {
    a.mu.Lock()        // genuine violation: never deferred
    b.mu.Lock()
    defer b.mu.Unlock()
    a.mu.Unlock()      // manual, non-deferred — SHOULD be flagged on a.mu.Lock()
}

Trace (key K = the shared field guarded.mu):

1. a.mu.Lock() → state{lockPos: aL} under K
2. b.mu.Lock() → same key K; prev.hasManualUnlock is false, so state is overwritten to {lockPos: bL} (a.mu's tracking is lost) — see lines 96-111
3. defer b.mu.Unlock() → state.hasDefer = true
4. a.mu.Unlock() → same key K → state.hasManualUnlock = true
5. Final check: hasManualUnlock && !hasDefer → true && !true → not reported

The real a.mu violation is masked by b.mu's defer.

Why it's order-dependent (and inconsistent)

When the manual unlock instead happens before the second instance's lock, the re-lock report path (lines 96-108) coincidentally fires and the violation is reported. So whether the same bug surfaces depends purely on statement ordering — a clear sign the identity model is wrong rather than merely incomplete.

Impact

• False negative (soundness hole in a CI-enforced linter): a non-deferred Unlock() that risks deadlock on panic ships unflagged whenever a sibling instance of the same struct type is locked-and-deferred in the same function.
• Methods that take multiple receivers/params of the same guarded type (common in merge/swap/compare/transfer helpers) are exactly the at-risk shape.

Recommendation

Distinguish receiver instances, not just the field declaration. For an *ast.SelectorExpr receiver, build a composite key from the base expression's identity plus the field — e.g. (pass.TypesInfo.ObjectOf(baseIdent), fieldObj) for a.mu, or fall back to the receiver expression's source text/position when the base is itself an expression. Single-instance cases (g.mu, local var mu) keep working; distinct instances no longer collide.

Before / after intent

• Before: a.mu and b.mu → key guarded.mu (collision)
• After: a.mu → (var a, field mu), b.mu → (var b, field mu) (independent tracking)

Validation checklist

• Add testdata with two same-typed struct instances (a, b *guarded) where one is manually unlocked and the other is deferred in the same function; assert the manual one is flagged
• Add the reverse-order variant to lock in order-independence
• Existing single-instance cases (g.mu, mu1/mu2 locals, nolint, RWMutex) still pass go test ./pkg/linters/manualmutexunlock/...

Effort

Small — localized to the key construction in getMutexReceiverObj plus a new testdata case.

Found via Serena semantic audit of never-deep-audited CI-enforced linters (R47).

Generated by 🤖 Sergo - Serena Go Expert · 260.8 AIC · ⌖ 11 AIC · ⊞ 5.9K · ◷

• expires on Jul 1, 2026, 9:12 PM UTC-08:00