From 3103fe97ea9bb2653c4890dcbe8a1d67d1016aac Mon Sep 17 00:00:00 2001 From: Stefan Haller Date: Fri, 3 Jul 2026 18:45:55 +0200 Subject: [PATCH] Replace RefreshingBranchesMutex with a branch-load sequence guard MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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) --- pkg/gui/controllers/helpers/refresh_helper.go | 28 +++++++++++++++++-- pkg/gui/types/common.go | 7 ++--- 2 files changed, 29 insertions(+), 6 deletions(-) diff --git a/pkg/gui/controllers/helpers/refresh_helper.go b/pkg/gui/controllers/helpers/refresh_helper.go index 95c7fa4a7..fe89bbad6 100644 --- a/pkg/gui/controllers/helpers/refresh_helper.go +++ b/pkg/gui/controllers/helpers/refresh_helper.go @@ -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. diff --git a/pkg/gui/types/common.go b/pkg/gui/types/common.go index 35f38d21e..2ce07f9c7 100644 --- a/pkg/gui/types/common.go +++ b/pkg/gui/types/common.go @@ -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