resourcetracker: manual cleanup in one branch is masked by defer in a sibling branch #62304

Summary

The shared pkg/linters/internal/resourcetracker package tracks each acquired resource with a single state{hasManual, hasDefer} entry keyed by the linter-provided key, populated by one flat ast.Inspect over the whole function body with no control-flow branch scoping (resourcetracker.go:89-112). When a single resource acquisition is followed by two mutually exclusive branches — one that cleans up manually, and a sibling branch (e.g. else) that defers the same cleanup — both cleanup calls write into the same per-key state entry. hasManual becomes true from one branch and hasDefer becomes true from the other, so the reporting guard hasManual && !hasDefer (resourcetracker.go:102) evaluates false and the manual-cleanup violation in the first branch is silently swallowed.

This is the same root-cause pattern found last run in httprespbodyclose.go (issue #62115, branch_state_merge), but here it lives in the shared resourcetracker framework, so it simultaneously affects all three linters built on it: manualmutexunlock (CI-enforced, native+wasm), fileclosenotdeferred, and contextcancelnotdeferred.

Repro (manualmutexunlock)

func F(cond bool) {
    var mu sync.Mutex
    mu.Lock()
    if cond {
        mu.Unlock()          // manual unlock, no defer protecting this path
    } else {
        defer mu.Unlock()
    }
    // work that could panic
}

Walking this body: mu.Lock() creates one tracked[mu] entry (resourcetracker.go:142). The if branch ExprStmt mu.Unlock() sets hasManual = true on that entry via markManual (resourcetracker.go:157-160). The else branch defer mu.Unlock() sets hasDefer = true on the same entry, same key, same acquisition (resourcetracker.go:146-152). At the end of inspectBody, tracked[mu] shows hasManual=true, hasDefer=true, so hasManual && !hasDefer is false and no diagnostic is reported, even though the cond==true path locks mu and unlocks it manually with zero panic protection, exactly the deadlock risk manualmutexunlock exists to catch.

The identical shape applies to fileclosenotdeferred (open file, if branch does f.Close() manually, else branch does defer f.Close()) and contextcancelnotdeferred (same pattern with cancel()), since both reuse the same resourcetracker.Config/inspectBody machinery with no per-linter override of the state model.

Root cause

resourcetracker.go:89-112 (inspectBody) uses one map[K]*state per function, populated by a single flat ast.Inspect call with no branch/path scoping. The state model implicitly assumes every acquisitions cleanup calls are on the same execution path, so it silently OR-merges facts about mutually exclusive branches as if they were cumulative facts about one linear execution, the same branch_state_merge class identified in httprespbodyclose.go (#62115), one layer up in a shared framework used by 3 linters instead of a standalone one.

Suggested fix

Track cleanup state per reachable branch instead of per acquisition-key-for-the-whole-function, e.g. walk IfStmt/SwitchStmt/SelectStmt bodies as separate sub-scopes that each inherit a copy of the parent state, and only merge back into the parent scope when the branch outcomes are actually compatible (both resolved, or genuinely sequential). At minimum, a cleanup call found inside one conditional arm must not retroactively resolve a violation recorded via a different, mutually exclusive arms manual call for the same key.

Validation checklist

• New testdata case per affected linter: single acquisition, manual cleanup in one if/else arm, deferred cleanup in the sibling arm, expect the manual arm to still be flagged.
• Existing testdata (e.g. BadTwoGuardsManualFirst, which uses distinct mutex instances, not branch-split cleanup of the same instance) continues to pass unchanged.
• Re-verify manualmutexunlock, fileclosenotdeferred, and contextcancelnotdeferred testdata together since all three share the fix location.

Impact

No live production trigger found via grep across pkg/ (real Lock()/os.Open/context.With* sites all use straight-line defer, not branch-split cleanup), so this is latent rather than currently causing missed diagnostics in this repo. But manualmutexunlock is CI-enforced (-test=false, native+wasm per .github/workflows/cgo.yml), and the gap is a genuine false negative on exactly the deadlock/leak/resource-lifecycle shape all three linters exist to catch.

Effort: medium, framework-level fix in one shared file, plus new testdata in three linter packages.

network:
  allowed:
    - defaults
    - "api.anthropic.com"

Generated by 🤖 Sergo - Serena Go Expert · claude · agent · 181.7 AIC · ⌖ 5.42 AIC · ⊞ 7K · ◷

• expires on Sep 27, 2026, 8:00 PM UTC-08:00