diff --git a/pkg/gui/controllers/helpers/merge_and_rebase_helper.go b/pkg/gui/controllers/helpers/merge_and_rebase_helper.go index c25416a3b..51488a922 100644 --- a/pkg/gui/controllers/helpers/merge_and_rebase_helper.go +++ b/pkg/gui/controllers/helpers/merge_and_rebase_helper.go @@ -307,6 +307,10 @@ func (self *MergeAndRebaseHelper) PromptToContinueRebase() error { // Need to refresh the files to be really sure if this is the case. // We would otherwise be relying on lazygit's auto-refresh on focus, // but this is not supported by all terminals or on all platforms. + // + // The model.Files update is bounced onto the UI thread, so we have + // to read it in Then; reading it inline here would see the previous + // model. self.c.Refresh(types.RefreshOptions{ Mode: types.SYNC, Scope: []types.RefreshableView{types.FILES}, Then: func() error { diff --git a/pkg/gui/controllers/helpers/refresh_helper.go b/pkg/gui/controllers/helpers/refresh_helper.go index 2933d9378..2f2210f49 100644 --- a/pkg/gui/controllers/helpers/refresh_helper.go +++ b/pkg/gui/controllers/helpers/refresh_helper.go @@ -238,7 +238,14 @@ func (self *RefreshHelper) Refresh(options types.RefreshOptions) { if scopeSet.Includes(types.STAGING) { refresh("staging", func() { fileWg.Wait() - self.stagingHelper.RefreshStagingPanel(types.OnFocusOpts{}) + // Bounce onto the UI thread so this runs after the files + // scope's model-update bounce — RefreshStagingPanel reads + // Model.Files (via Files.GetSelected) and would otherwise + // see the pre-refresh model. + self.c.OnUIThread(func() error { + self.stagingHelper.RefreshStagingPanel(types.OnFocusOpts{}) + return nil + }) }) } @@ -667,15 +674,8 @@ func (self *RefreshHelper) refreshTags() error { return nil } -func (self *RefreshHelper) refreshStateSubmoduleConfigs() error { - configs, err := self.c.Git().Submodule.GetConfigs(nil) - if err != nil { - return err - } - - self.c.Model().Submodules = configs - - return nil +func (self *RefreshHelper) refreshStateSubmoduleConfigs() ([]*models.SubmoduleConfig, error) { + return self.c.Git().Submodule.GetConfigs(nil) } // self.refreshStatus is called at the end of this because that's when we can @@ -743,25 +743,40 @@ func (self *RefreshHelper) refreshFilesAndSubmodules(background bool) error { self.c.Mutexes().RefreshingFilesMutex.Lock() defer self.c.Mutexes().RefreshingFilesMutex.Unlock() - if err := self.refreshStateSubmoduleConfigs(); err != nil { + configs, err := self.refreshStateSubmoduleConfigs() + if err != nil { return err } - if err := self.refreshStateFiles(background); err != nil { + if err := self.refreshStateFiles(background, configs); err != nil { return err } - self.c.OnUIThread(func() error { - self.refreshView(self.c.Contexts().Submodules) - self.refreshView(self.c.Contexts().Files) - return nil - }) + self.refreshView(self.c.Contexts().Submodules) + self.refreshView(self.c.Contexts().Files) return nil } -func (self *RefreshHelper) refreshStateFiles(background bool) error { +// onUIThreadUnlessRepoChanged bounces a refresh's model/view update onto the UI +// thread, but drops it if the repo was switched while the refresh was in flight. +// Refresh workers do their git work off the UI thread and enqueue their model +// writes here; a repo switch (which replaces the whole model and context tree) +// bumps the generation, so a write captured under the old generation must not +// clobber the new repo's state. Callers capture the generation with +// State().GetRepoGeneration() before doing their git work and pass it in. +func (self *RefreshHelper) onUIThreadUnlessRepoChanged(generation int, f func() error) { + self.c.OnUIThread(func() error { + if self.c.State().GetRepoGeneration() != generation { + return nil + } + return f() + }) +} + +func (self *RefreshHelper) refreshStateFiles(background bool, submoduleConfigs []*models.SubmoduleConfig) error { fileTreeViewModel := self.c.Contexts().Files.FileTreeViewModel + generation := self.c.State().GetRepoGeneration() prevConflictFileCount := 0 if self.c.UserConfig().Git.AutoStageResolvedConflicts { @@ -822,7 +837,9 @@ func (self *RefreshHelper) refreshStateFiles(background bool) error { // (e.g. in the user's editor). Offer to continue it. We only do this // for operations we started ourselves; prompting for one that was // started outside lazygit (e.g. by a coding agent) would be confusing. - self.c.OnUIThread(func() error { return self.mergeAndRebaseHelper.PromptToContinueRebase() }) + self.onUIThreadUnlessRepoChanged(generation, func() error { + return self.mergeAndRebaseHelper.PromptToContinueRebase() + }) } } else { // Either there's no operation in progress any more, or new conflicts have @@ -835,22 +852,23 @@ func (self *RefreshHelper) refreshStateFiles(background bool) error { }) } - fileTreeViewModel.RWMutex.Lock() - - // only taking over the filter if it hasn't already been set by the user. - if conflictFileCount > 0 && prevConflictFileCount == 0 { - if fileTreeViewModel.GetStatusFilter() == filetree.DisplayAll { - fileTreeViewModel.SetStatusFilter(filetree.DisplayConflicted) - self.c.Contexts().Files.GetView().Subtitle = self.c.Tr.FilterLabelConflictingFiles + self.onUIThreadUnlessRepoChanged(generation, func() error { + // only taking over the filter if it hasn't already been set by the user. + if conflictFileCount > 0 && prevConflictFileCount == 0 { + if fileTreeViewModel.GetStatusFilter() == filetree.DisplayAll { + fileTreeViewModel.SetStatusFilter(filetree.DisplayConflicted) + self.c.Contexts().Files.GetView().Subtitle = self.c.Tr.FilterLabelConflictingFiles + } + } else if conflictFileCount == 0 && fileTreeViewModel.GetStatusFilter() == filetree.DisplayConflicted { + fileTreeViewModel.SetStatusFilter(filetree.DisplayAll) + self.c.Contexts().Files.GetView().Subtitle = "" } - } else if conflictFileCount == 0 && fileTreeViewModel.GetStatusFilter() == filetree.DisplayConflicted { - fileTreeViewModel.SetStatusFilter(filetree.DisplayAll) - self.c.Contexts().Files.GetView().Subtitle = "" - } - self.c.Model().Files = files - fileTreeViewModel.SetTree() - fileTreeViewModel.RWMutex.Unlock() + self.c.Model().Submodules = submoduleConfigs + self.c.Model().Files = files + fileTreeViewModel.SetTree() + return nil + }) return nil } diff --git a/pkg/gui/gui.go b/pkg/gui/gui.go index 87a87f7b2..834cf1e2d 100644 --- a/pkg/gui/gui.go +++ b/pkg/gui/gui.go @@ -12,6 +12,7 @@ import ( "sort" "strings" "sync" + "sync/atomic" "time" "github.com/jesseduffield/lazycore/pkg/boxlayout" @@ -111,6 +112,11 @@ type Gui struct { PopupHandler types.IPopupHandler + // Bumped every time we switch to a different repository (in resetState). + // Used to drop refresh results that were computed for a repo we've since + // navigated away from. See RefreshHelper.onUIThreadUnlessRepoChanged. + repoGeneration atomic.Int32 + // we use this to decide whether we'll return to the original directory that // lazygit was opened in, or if we'll retain the one we're currently in. RetainOriginalDir bool @@ -169,6 +175,10 @@ func (self *StateAccessor) GetRepoState() types.IRepoStateAccessor { return self.gui.State } +func (self *StateAccessor) GetRepoGeneration() int { + return int(self.gui.repoGeneration.Load()) +} + func (self *StateAccessor) GetPagerConfig() *config.PagerConfig { return self.gui.pagerConfig } @@ -575,6 +585,11 @@ func (gui *Gui) checkForChangedConfigsThatDontAutoReload(oldConfig *config.UserC // resetState reuses the repo state from our repo state map, if the repo was // open before; otherwise it creates a new one. func (gui *Gui) resetState(startArgs appTypes.StartArgs) types.Context { + // Bump the repo generation so that any refresh still in flight for the + // previous repo drops its model update instead of applying it here (see + // RefreshHelper.onUIThreadUnlessRepoChanged). + gui.repoGeneration.Add(1) + // Un-highlight the current view if there is one. The reason we do this is // that the repo we are switching to might have a different view focused, // and would then show an inactive highlight for the previous view. diff --git a/pkg/gui/types/common.go b/pkg/gui/types/common.go index ad28972ae..7fc75aee0 100644 --- a/pkg/gui/types/common.go +++ b/pkg/gui/types/common.go @@ -384,6 +384,13 @@ type IStateAccessor interface { GetItemOperation(item HasUrn) ItemOperation SetItemOperation(item HasUrn, operation ItemOperation) ClearItemOperation(item HasUrn) + + // A counter that is bumped every time we switch to a different repository + // (see Gui.resetState). Refresh workers capture it before doing their git + // work and pass it to onUIThreadUnlessRepoChanged, so that a model update + // computed for one repo can be dropped rather than applied to another if the + // user switched repos while the refresh was in flight. + GetRepoGeneration() int } type IRepoStateAccessor interface {