Skip to content

fix(manualmutexunlock): distinguish struct instances sharing a mutex field - #41383

Merged
pelikhan merged 2 commits into
mainfrom
copilot/fix-manualmutexunlock-issue
Jun 25, 2026
Merged

fix(manualmutexunlock): distinguish struct instances sharing a mutex field#41383
pelikhan merged 2 commits into
mainfrom
copilot/fix-manualmutexunlock-issue

Conversation

Copilot AI commented Jun 25, 2026

Copy link
Copy Markdown
Contributor

manualmutexunlock tracked mutex state in map[types.Object]*mutexVarState, keyed via pass.TypesInfo.Selections[r].Obj() for field selectors. That returns the field declaration — one *types.Var shared across all instances of the struct — so a.mu and b.mu collide on the same key and clobber each other's state. A deferred unlock on b.mu silently masks a manual-unlock violation on a.mu.

Changes

  • New mutexKey composite type {base, field types.Object} replaces bare types.Object as the map key.
    • Direct local (mu) → {base: ObjectOf(mu), field: nil} — unchanged semantics
    • Field selector (a.mu) → {base: ObjectOf(a), field: fieldObj} — instance-distinct
    • Non-ident base (getGuard().mu) → {base: fieldObj, field: nil} — fallback, prior behaviour preserved
  • getMutexReceiverObjgetMutexReceiverKey returns (mutexKey, bool) instead of types.Object.
  • getLockCallObj/getUnlockCallObjgetLockCallKey/getUnlockCallKey updated accordingly; all call sites in inspectMutexNode updated.
  • New testdata cases: BadTwoGuardsManualFirst / BadTwoGuardsManualSecond assert the violation is flagged regardless of statement order; GoodTwoGuardsBothDeferred confirms no false positive when both are deferred.
func BadTwoGuardsManualFirst(a, b *guarded) {
    a.mu.Lock()        // flagged — manual unlock, no defer
    b.mu.Lock()
    defer b.mu.Unlock()
    a.mu.Unlock()      // before fix: b's defer masked this; now correctly reported
}

…uct instances sharing a mutex field

Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Copilot AI changed the title [WIP] Fix manualmutexunlock false negative issue fix(manualmutexunlock): distinguish struct instances sharing a mutex field Jun 25, 2026
Copilot AI requested a review from pelikhan June 25, 2026 05:48
@pelikhan
pelikhan marked this pull request as ready for review June 25, 2026 05:52
Copilot AI review requested due to automatic review settings June 25, 2026 05:52
@pelikhan
pelikhan merged commit b0b807c into main Jun 25, 2026
1 check passed
@pelikhan
pelikhan deleted the copilot/fix-manualmutexunlock-issue branch June 25, 2026 05:52

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

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 fixes a correctness issue in the manualmutexunlock analyzer where mutex state tracked for struct field selectors (e.g. a.mu) could collide across different struct instances because the key was derived from the shared field declaration object. The update introduces an instance-aware key so violations are reported reliably.

Changes:

  • Replace the mutex tracking map key from types.Object to a composite mutexKey{base, field} to distinguish a.mu vs b.mu.
  • Update receiver/call extraction helpers to return (mutexKey, bool) and wire through all call sites.
  • Add new testdata cases covering two-guard scenarios to prevent regressions.
Show a summary per file
File Description
pkg/linters/manualmutexunlock/manualmutexunlock.go Switch tracking to an instance-aware key and update receiver resolution helpers.
pkg/linters/manualmutexunlock/testdata/src/manualmutexunlock/manualmutexunlock.go Add regression tests ensuring separate struct instances don’t clobber each other’s mutex state.

Copilot's findings

Tip

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

  • Files reviewed: 2/2 changed files
  • Comments generated: 1

Comment on lines +202 to 217
fieldObj := sel.Obj()
// When the base is a plain identifier (the common case: `a.mu`),
// build a composite key (base var, field) so that distinct
// instances of the same struct type are tracked independently.
baseIdent, ok := r.X.(*ast.Ident)
if !ok {
// Fall back for non-ident base expressions (e.g. `getGuard().mu`):
// use the field object alone as the key, matching prior behaviour.
return mutexKey{base: fieldObj}, true
}
baseObj := pass.TypesInfo.ObjectOf(baseIdent)
if baseObj == nil {
return mutexKey{base: fieldObj}, true
}
return mutexKey{base: baseObj, field: fieldObj}, true
}
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.

3 participants