From 5fc678dde6ea9dad188f95293de75da3be9779bc Mon Sep 17 00:00:00 2001 From: Stefan Haller Date: Mon, 20 Jul 2026 15:59:36 +0200 Subject: [PATCH] Read the gui's per-repo pointers on the UI thread in background routines gui.git, gui.helpers and gui.State are all replaced on a repo switch, which runs on the UI thread. The background fetch and the external- change poller read them from their own goroutines, racing the reassignment. This race can't show up in the integration suite, which doesn't enable the background routines, so no -race run will ever flag it; it can only bite real users who switch repos while a background fetch or poll is in flight. Capture the objects a routine iteration needs in a single blocking UI-thread hop before using them, the same pattern the refresh's input capture uses. For the fetch this has two welcome side effects: the fetch, the post-fetch refresh's generation baseline, and the recorded fetch time now all refer to the same repo (the old comment documented the timestamp's mismatch as a known, unguarded race), and the git instance the fetch runs through is pinned to that repo's directory, so a switch mid-fetch can no longer direct in-flight work at the new repo. Co-Authored-By: Claude Fable 5 --- pkg/gui/background.go | 66 ++++++++++++++++++++++++++++++------------- 1 file changed, 46 insertions(+), 20 deletions(-) diff --git a/pkg/gui/background.go b/pkg/gui/background.go index d39dfd84c..180b2d445 100644 --- a/pkg/gui/background.go +++ b/pkg/gui/background.go @@ -6,7 +6,9 @@ import ( "sync/atomic" "time" + "github.com/jesseduffield/lazygit/pkg/commands" "github.com/jesseduffield/lazygit/pkg/gocui" + "github.com/jesseduffield/lazygit/pkg/gui/controllers/helpers" "github.com/jesseduffield/lazygit/pkg/gui/types" "github.com/jesseduffield/lazygit/pkg/utils" ) @@ -106,23 +108,35 @@ func (self *BackgroundRoutineMgr) startBackgroundFetch() { self.gui.waitForIntro.Wait() fetch := func(firstTimeOrRetriggered bool) error { - // Do this on the UI thread so that we don't have to deal with synchronization around the - // access of the repo state. - self.gui.onUIThread(func() error { - // There's a race here, where we might be recording the time stamp for a different repo - // than where the fetch actually ran. It's not very likely though, and not harmful if it - // does happen; guarding against it would be more effort than it's worth. + // Capture what the fetch needs from the gui's per-repo state in a + // single UI-thread hop: gui.git, gui.helpers and gui.State are all + // replaced on a repo switch (which runs on the UI thread), so reading + // them from this background goroutine would race the reassignment. + // Capturing them together also ties the fetch, the post-fetch + // refresh's generation baseline, and the recorded fetch time to the + // same repo. + var git *commands.GitCommand + var appStatusHelper *helpers.AppStatusHelper + var branchesHelper *helpers.BranchesHelper + var fetchGeneration int + if err := self.gui.g.OnUIThreadAndWaitBackground(func() error { + git = self.gui.git + appStatusHelper = self.gui.helpers.AppStatus + branchesHelper = self.gui.helpers.BranchesHelper + fetchGeneration = self.gui.c.State().GetRepoGeneration() self.gui.State.LastBackgroundFetchTime = time.Now() return nil - }) + }); err != nil { + return err + } if self.gui.UserConfig().Gui.ShowBottomLine || firstTimeOrRetriggered { - return self.gui.helpers.AppStatus.WithWaitingStatusImpl(self.gui.Tr.FetchingStatus, func(gocui.Task) error { - return self.backgroundFetch() + return appStatusHelper.WithWaitingStatusImpl(self.gui.Tr.FetchingStatus, func(gocui.Task) error { + return self.backgroundFetch(git, branchesHelper, fetchGeneration) }, nil) } - return self.backgroundFetch() + return self.backgroundFetch(git, branchesHelper, fetchGeneration) } // We want an immediate fetch at startup, and since goEvery starts by @@ -165,7 +179,20 @@ func (self *BackgroundRoutineMgr) startBackgroundExternalChangeDetection() { } func (self *BackgroundRoutineMgr) checkForExternalChanges() { - current, err := self.gui.git.Status.RefsSnapshot() + // Capture the per-repo objects in a UI-thread hop, like the background + // fetch does: gui.git and gui.helpers are replaced on a repo switch, so + // reading them from this background goroutine would race the reassignment. + var git *commands.GitCommand + var refreshHelper *helpers.RefreshHelper + if err := self.gui.g.OnUIThreadAndWaitBackground(func() error { + git = self.gui.git + refreshHelper = self.gui.helpers.Refresh + return nil + }); err != nil { + return + } + + current, err := git.Status.RefsSnapshot() if err != nil { // Transient error (e.g. git process couldn't start). Don't update the // stored snapshot; we'll retry next tick. @@ -173,7 +200,7 @@ func (self *BackgroundRoutineMgr) checkForExternalChanges() { return } - if !self.gui.helpers.Refresh.RefsSnapshotChangedSince(current) { + if !refreshHelper.RefsSnapshotChangedSince(current) { return } @@ -231,15 +258,14 @@ func (self *BackgroundRoutineMgr) goEvery(interval time.Duration, stop, retrigge }) } -func (self *BackgroundRoutineMgr) backgroundFetch() (err error) { - // Captured before the fetch, not after: the fetch is a network call during - // which the user may switch repos, and the post-fetch refresh needs to be - // able to tell (see PostFetchRefresh). - fetchGeneration := self.gui.c.State().GetRepoGeneration() +// The parameters are captured by the caller before the fetch starts, not read +// here after it: the fetch is a network call during which the user may switch +// repos, and the post-fetch refresh needs to be able to tell (see +// PostFetchRefresh). +func (self *BackgroundRoutineMgr) backgroundFetch(git *commands.GitCommand, branchesHelper *helpers.BranchesHelper, fetchGeneration int) error { + err := git.Sync.FetchBackground() - err = self.gui.git.Sync.FetchBackground() - - return self.gui.helpers.BranchesHelper.PostFetchRefresh(err, true, fetchGeneration) + return branchesHelper.PostFetchRefresh(err, true, fetchGeneration) } func (self *BackgroundRoutineMgr) triggerImmediateFetch() {