Remove refresh mutexes made redundant by bouncing

Now that every refresh scope writes its model updates on the UI thread
via onUIThreadUnlessRepoChanged, the per-scope mutexes that used to
serialize concurrent worker-goroutine access are redundant:
Model().Commits, .SubCommits, .Authors, the status view content, and
.PullRequests/.PullRequestsMap are all now written only on the UI
thread, and their readers already ran there. setSubCommits only existed
to take the lock, so it's inlined to match refreshSubCommitsWithLimit,
which writes Model().SubCommits directly.

The worker phases still *read* some of these fields (the commit
selection range, MergeRebasingCommits), but those reads race a
concurrent refresh's bounced write regardless of the mutex — the write
happens in the bounce, outside the locked region — so the mutex never
protected them. That residual read race belongs to the broader -race
effort, not to these locks.

RefreshingBranchesMutex is deliberately kept. It is load-bearing for a
reason unrelated to data races: at the INITIAL startup stage two
refreshBranches run concurrently — an immediate one with an empty
reflog (non-recency order) and an async one with the freshly-loaded
reflog (recency order). The mutex serializes them so the recency write's
bounce is enqueued last and wins. Without it the stale non-recency write
can land last, reordering the branches list (caught by the recency-sort
e2e tests: cherry_pick/*, branch/rebase_*).

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
Stefan Haller 2026-07-03 16:51:55 +02:00
parent f7a61443fa
commit 805738034f
3 changed files with 5 additions and 37 deletions

View file

@ -472,9 +472,6 @@ func (self *RefreshHelper) determineCheckedOutRef() models.Ref {
}
func (self *RefreshHelper) refreshCommitsWithLimit(commitSelection types.CommitSelectionBehavior) error {
self.c.Mutexes().LocalCommitsMutex.Lock()
defer self.c.Mutexes().LocalCommitsMutex.Unlock()
generation := self.c.State().GetRepoGeneration()
var selectionRange *localCommitSelectionRange
@ -629,9 +626,6 @@ func (self *RefreshHelper) refreshSubCommitsWithLimit() error {
return nil
}
self.c.Mutexes().SubCommitsMutex.Lock()
defer self.c.Mutexes().SubCommitsMutex.Unlock()
generation := self.c.State().GetRepoGeneration()
commits, err := self.c.Git().Loaders.CommitLoader.GetCommits(
@ -661,9 +655,6 @@ func (self *RefreshHelper) refreshSubCommitsWithLimit() error {
}
func (self *RefreshHelper) RefreshAuthors(commits []*models.Commit) {
self.c.Mutexes().AuthorsMutex.Lock()
defer self.c.Mutexes().AuthorsMutex.Unlock()
authors := self.c.Model().Authors
for _, commit := range commits {
if _, ok := authors[commit.AuthorEmail]; !ok {
@ -694,9 +685,6 @@ func (self *RefreshHelper) refreshCommitFilesContext() error {
}
func (self *RefreshHelper) refreshRebaseCommits() error {
self.c.Mutexes().LocalCommitsMutex.Lock()
defer self.c.Mutexes().LocalCommitsMutex.Unlock()
generation := self.c.State().GetRepoGeneration()
updatedCommits, err := self.c.Git().Loaders.CommitLoader.MergeRebasingCommits(self.c.Model().HashPool, self.c.Model().Commits)
@ -795,9 +783,7 @@ func (self *RefreshHelper) refreshBranches(refreshWorktrees bool, keepBranchSele
// Need to re-render the commits view because the visualization of local
// branch heads might have changed
self.c.Mutexes().LocalCommitsMutex.Lock()
self.c.Contexts().LocalCommits.HandleRender()
self.c.Mutexes().LocalCommitsMutex.Unlock()
return nil
})
@ -1075,9 +1061,6 @@ func (self *RefreshHelper) refreshStashEntries() {
// never call this on its own, it should only be called from within refreshCommits()
func (self *RefreshHelper) refreshStatus() {
self.c.Mutexes().RefreshingStatusMutex.Lock()
defer self.c.Mutexes().RefreshingStatusMutex.Unlock()
generation := self.c.State().GetRepoGeneration()
workingTreeState := self.c.Git().Status.WorkingTreeState()
@ -1147,9 +1130,6 @@ func (self *RefreshHelper) refreshView(context types.Context) {
}
func (self *RefreshHelper) refreshGithubPullRequests() {
self.c.Mutexes().RefreshingPullRequestsMutex.Lock()
defer self.c.Mutexes().RefreshingPullRequestsMutex.Unlock()
generation := self.c.State().GetRepoGeneration()
clearPullRequests := func() {

View file

@ -49,7 +49,7 @@ func (self *SubCommitsHelper) ViewSubCommits(opts ViewSubCommitsOpts) error {
return err
}
self.setSubCommits(commits)
self.c.Model().SubCommits = commits
self.refreshHelper.RefreshAuthors(commits)
subCommitsContext := self.c.Contexts().SubCommits
@ -71,10 +71,3 @@ func (self *SubCommitsHelper) ViewSubCommits(opts ViewSubCommitsOpts) error {
self.c.Context().Push(self.c.Contexts().SubCommits, types.OnFocusOpts{})
return nil
}
func (self *SubCommitsHelper) setSubCommits(commits []*models.Commit) {
self.c.Mutexes().SubCommitsMutex.Lock()
defer self.c.Mutexes().SubCommitsMutex.Unlock()
self.c.Model().SubCommits = commits
}

View file

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