Replace RefreshingBranchesMutex with a branch-load sequence guard

This removes the last refresh mutex. RefreshingBranchesMutex wasn't
guarding a data race (Branch.BehindBaseBranch is atomic, and every model
write is now bounced onto the UI thread); it was serializing the two
branch loads that race at the INITIAL startup stage — an immediate one
sorted without the reflog, and an async one that loads the reflog and
sorts by recency — so that the recency-sorted write landed last and won.
That serialization was never a real guarantee, only "very likely": it
relied on the immediate load acquiring the lock before the async load,
which had to load the reflog first.

Instead, each branch load takes a monotonically increasing sequence
number, and its bounce drops the write if a later-started load has
already applied. Combined with the preceding commit (immediate load runs
before the async one is spawned), this is an actual guarantee: the
immediate non-recency load always has a lower sequence than its recency
async partner, so the highest sequence number is always held by a
recency-sorted load, and highest-wins converges on recency ordering —
even if more refreshes fire during the INITIAL window, since each
refresh's async out-sequences its own immediate.

The guard also subsumes what the mutex gave post-startup: a slow, stale
refresh's bounce can no longer clobber a newer refresh's branches.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
Stefan Haller 2026-07-03 18:45:55 +02:00
parent 6d8ab1d063
commit 3103fe97ea
2 changed files with 29 additions and 6 deletions

View file

@ -3,6 +3,7 @@ package helpers
import (
"strings"
"sync"
"sync/atomic"
"time"
"github.com/jesseduffield/generics/set"
@ -44,6 +45,15 @@ type RefreshHelper struct {
// refresh that re-read refs/commits, read by the poller.
refsSnapshotMutex deadlock.Mutex
refsSnapshot string
// branchLoadSeq hands out a monotonically increasing sequence number to
// each branch load (via Add, on the worker); appliedBranchLoadSeq is the
// highest sequence whose result has been written to the model (touched only
// on the UI thread, inside the bounce). Together they let a branch load's
// bounce drop its write if a later-started load has already applied, so
// concurrent branch loads don't clobber each other out of order.
branchLoadSeq atomic.Int64
appliedBranchLoadSeq int64
}
func NewRefreshHelper(
@ -384,6 +394,11 @@ func getModeName(mode types.RefreshMode) string {
// i.e. not by recency), then load the reflog on a worker and refresh the
// branches again, this time recency-sorted. From then on we're in the COMPLETE
// phase and load the reflog synchronously before refreshing the branches.
//
// The immediate refresh must run before we spawn the async one, not after: that
// order gives the immediate (non-recency) load a lower branch-load sequence
// than the async (recency) load, so the sequence guard in refreshBranches keeps
// the recency-sorted result even if the two loads' bounces land out of order.
func (self *RefreshHelper) refreshReflogAndBranches(refreshWorktrees bool, keepBranchSelectionIndex bool) {
switch self.c.State().GetRepoState().GetStartupStage() {
case types.INITIAL:
@ -714,8 +729,7 @@ func (self *RefreshHelper) refreshStateSubmoduleConfigs() ([]*models.SubmoduleCo
// self.refreshStatus is called at the end of this because that's when we can
// be sure there is a State.Model.Branches array to pick the current branch from
func (self *RefreshHelper) refreshBranches(refreshWorktrees bool, keepBranchSelectionIndex bool, loadBehindCounts bool, reflogCommits []*models.Commit) {
self.c.Mutexes().RefreshingBranchesMutex.Lock()
defer self.c.Mutexes().RefreshingBranchesMutex.Unlock()
loadSeq := self.branchLoadSeq.Add(1)
generation := self.c.State().GetRepoGeneration()
@ -748,6 +762,16 @@ func (self *RefreshHelper) refreshBranches(refreshWorktrees bool, keepBranchSele
}
self.onUIThreadUnlessRepoChanged(generation, func() error {
// Drop this write if a branch load that started later has already applied
// its result. At the INITIAL startup stage an immediate load (not
// recency-sorted) and an async recency-sorted load run concurrently; this
// makes the later-started (recency-sorted) one win regardless of which
// finishes first, so its result isn't clobbered by the stale immediate one.
if loadSeq < self.appliedBranchLoadSeq {
return nil
}
self.appliedBranchLoadSeq = loadSeq
self.c.Model().Branches = branches
// Rebuilding here (rather than on the worker) means the map is built from
// the branches we just wrote, on the UI thread.

View file

@ -338,10 +338,9 @@ type Model struct {
}
type Mutexes struct {
RefreshingBranchesMutex deadlock.Mutex
SubprocessMutex deadlock.Mutex
PopupMutex deadlock.Mutex
PtyMutex deadlock.Mutex
SubprocessMutex deadlock.Mutex
PopupMutex deadlock.Mutex
PtyMutex deadlock.Mutex
}
// A long-running operation associated with an item. For example, we'll show