From 080542c9fb1e3eebbbcc1ea3fd5d8e6f064527db Mon Sep 17 00:00:00 2001 From: Stefan Haller Date: Mon, 6 Jul 2026 14:45:03 +0200 Subject: [PATCH] Capture the commits refresh's inputs on the UI thread MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A commits refresh does its git work on a worker and then reads the model, the contexts, and the modes for that work directly from there: LocalCommits.GetSelectionRangeAndMode/GetLimitCommits/GetShowWholeGitGraph, Model.Commits/MainBranches/HashPool, the filtering path/author. Those are owned by the UI thread, which is concurrently running the cursor and render code, so the reads race it — the dominant, confirmed source of the commits-scope flakes (the startup ClampSelection vs GetSelectionRangeAndMode race, for one). Gather them into an immutable capturedCommitState on the UI thread, before the git work is dispatched, and have refreshCommitsWithLimit compute from that snapshot. UI-thread callers capture inline; worker callers can't (a SYNC/BLOCK_UI refresh parks the UI thread at wg.Wait, so hopping from a scope sub-worker would deadlock), so the capture is lifted out of the scope worker into the refresh orchestration, and worker callers announce themselves with a new RefreshFromWorker entry point that hops the capture to the UI thread and blocks for it (OnUIThreadAndWait). BLOCK_UI runs the whole refresh on the UI thread regardless of the caller, so it captures inline too. Every refresh issued from a worker that reaches the commits (or branches, which pulls in commits) scope is converted: the fast-forward, branch/tag delete, worktree remove/detach, push, reword-via-rebase, author edits, custom-command, hard-reset-with-autostash, reset-to-ref, fetch-and-checkout, gpg-stream, post-fetch, and external-change-poller refreshes, plus the branch checkout and move-commits-to-new-branch refreshes. Co-Authored-By: Claude Opus 4.8 (1M context) --- pkg/gui/background.go | 2 +- pkg/gui/controllers/branches_controller.go | 4 +- .../controllers/helpers/branches_helper.go | 12 +- pkg/gui/controllers/helpers/gpg_helper.go | 4 +- pkg/gui/controllers/helpers/refresh_helper.go | 130 +++++++++++++++--- pkg/gui/controllers/helpers/refs_helper.go | 8 +- .../controllers/helpers/worktree_helper.go | 4 +- .../controllers/local_commits_controller.go | 8 +- pkg/gui/controllers/remotes_controller.go | 2 +- pkg/gui/controllers/sync_controller.go | 2 +- pkg/gui/controllers/tags_controller.go | 6 +- pkg/gui/controllers/undo_controller.go | 2 +- pkg/gui/gui_common.go | 4 + .../custom_commands/handler_creator.go | 2 +- pkg/gui/types/common.go | 6 + 15 files changed, 147 insertions(+), 49 deletions(-) diff --git a/pkg/gui/background.go b/pkg/gui/background.go index 6215f0d45..17f3677f6 100644 --- a/pkg/gui/background.go +++ b/pkg/gui/background.go @@ -184,7 +184,7 @@ func (self *BackgroundRoutineMgr) checkForExternalChanges() { // No need to update the stored snapshot here; Refresh does that. self.gui.c.Log.Info("External ref change detected — refreshing") - self.gui.c.Refresh(types.RefreshOptions{Background: true}) + self.gui.c.RefreshFromWorker(types.RefreshOptions{Background: true}) } // returns a channel that can be used to trigger the callback immediately diff --git a/pkg/gui/controllers/branches_controller.go b/pkg/gui/controllers/branches_controller.go index a5c55884b..9b9e9e546 100644 --- a/pkg/gui/controllers/branches_controller.go +++ b/pkg/gui/controllers/branches_controller.go @@ -734,7 +734,7 @@ func (self *BranchesController) fastForward(branch *models.Branch) error { WorktreePath: worktreePath, }, ) - self.c.Refresh(types.RefreshOptions{Mode: types.SYNC}) + self.c.RefreshFromWorker(types.RefreshOptions{Mode: types.SYNC}) return err } @@ -743,7 +743,7 @@ func (self *BranchesController) fastForward(branch *models.Branch) error { err := self.c.Git().Sync.FastForward( task, branch.Name, branch.UpstreamRemote, branch.UpstreamBranch, ) - self.c.Refresh(types.RefreshOptions{Mode: types.SYNC, Scope: []types.RefreshableView{types.BRANCHES}}) + self.c.RefreshFromWorker(types.RefreshOptions{Mode: types.SYNC, Scope: []types.RefreshableView{types.BRANCHES}}) return err }) } diff --git a/pkg/gui/controllers/helpers/branches_helper.go b/pkg/gui/controllers/helpers/branches_helper.go index 053804760..683d26db3 100644 --- a/pkg/gui/controllers/helpers/branches_helper.go +++ b/pkg/gui/controllers/helpers/branches_helper.go @@ -46,7 +46,7 @@ func (self *BranchesHelper) ConfirmLocalDelete(branches []*models.Branch) error } self.c.Contexts().Branches.CollapseRangeSelectionToTop() - self.c.Refresh(types.RefreshOptions{Mode: types.ASYNC, Scope: []types.RefreshableView{types.BRANCHES}}) + self.c.RefreshFromWorker(types.RefreshOptions{Mode: types.ASYNC, Scope: []types.RefreshableView{types.BRANCHES}}) return nil }) }) @@ -84,7 +84,7 @@ func (self *BranchesHelper) ConfirmDeleteRemote(remoteBranches []*models.RemoteB if err := self.deleteRemoteBranches(remoteBranches, task); err != nil { return err } - self.c.Refresh(types.RefreshOptions{Mode: types.ASYNC, Scope: []types.RefreshableView{types.BRANCHES, types.REMOTES}}) + self.c.RefreshFromWorker(types.RefreshOptions{Mode: types.ASYNC, Scope: []types.RefreshableView{types.BRANCHES, types.REMOTES}}) if resetRemoteBranchesSelection { self.c.Contexts().RemoteBranches.CollapseRangeSelectionToTop() } @@ -152,7 +152,7 @@ func (self *BranchesHelper) ConfirmLocalAndRemoteDelete(branches []*models.Branc } self.c.Contexts().Branches.CollapseRangeSelectionToTop() - self.c.Refresh(types.RefreshOptions{Mode: types.ASYNC, Scope: []types.RefreshableView{types.BRANCHES, types.REMOTES}}) + self.c.RefreshFromWorker(types.RefreshOptions{Mode: types.ASYNC, Scope: []types.RefreshableView{types.BRANCHES, types.REMOTES}}) return nil }) }, @@ -312,7 +312,7 @@ func (self *BranchesHelper) deleteLocalBranchesContinuation(branches []*models.B } self.c.Contexts().Branches.CollapseRangeSelectionToTop() - self.c.Refresh(types.RefreshOptions{ + self.c.RefreshFromWorker(types.RefreshOptions{ Mode: types.ASYNC, Scope: []types.RefreshableView{types.WORKTREES, types.BRANCHES, types.FILES}, }) @@ -330,7 +330,7 @@ func (self *BranchesHelper) deleteLocalAndRemoteBranchesContinuation(branches [] } self.c.Contexts().Branches.CollapseRangeSelectionToTop() - self.c.Refresh(types.RefreshOptions{ + self.c.RefreshFromWorker(types.RefreshOptions{ Mode: types.ASYNC, Scope: []types.RefreshableView{types.WORKTREES, types.BRANCHES, types.REMOTES, types.FILES}, }) @@ -390,7 +390,7 @@ func (self *BranchesHelper) PostFetchRefresh(fetchErr error, background bool) er // AutoForwardBranches reads Model.Branches, which the branches refresh writes // via a bounce, so it has to run in Then rather than right after Refresh // returns (where it would still see the previous branches). - self.c.Refresh(types.RefreshOptions{ + self.c.RefreshFromWorker(types.RefreshOptions{ Scope: scope, Mode: types.SYNC, Background: background, diff --git a/pkg/gui/controllers/helpers/gpg_helper.go b/pkg/gui/controllers/helpers/gpg_helper.go index fb8fae628..fd74a400b 100644 --- a/pkg/gui/controllers/helpers/gpg_helper.go +++ b/pkg/gui/controllers/helpers/gpg_helper.go @@ -88,7 +88,7 @@ func (self *GpgHelper) runAndStream( ) error { return self.c.WithWaitingStatus(waitingStatus, func(gocui.Task) error { if err := cmdObj.StreamOutput().Run(); err != nil { - self.c.Refresh(failureRefreshOptions) + self.c.RefreshFromWorker(failureRefreshOptions) return fmt.Errorf( self.c.Tr.GitCommandFailed, self.c.UserConfig().Keybinding.Universal.ExtrasMenu, ) @@ -100,7 +100,7 @@ func (self *GpgHelper) runAndStream( } } - self.c.Refresh(successRefreshOptions) + self.c.RefreshFromWorker(successRefreshOptions) return nil }) } diff --git a/pkg/gui/controllers/helpers/refresh_helper.go b/pkg/gui/controllers/helpers/refresh_helper.go index 5e15e17c1..48a6b6db8 100644 --- a/pkg/gui/controllers/helpers/refresh_helper.go +++ b/pkg/gui/controllers/helpers/refresh_helper.go @@ -79,6 +79,17 @@ func NewRefreshHelper( } func (self *RefreshHelper) Refresh(options types.RefreshOptions) { + self.performRefresh(options, false) +} + +// RefreshFromWorker is Refresh for callers already running on a worker +// goroutine (e.g. inside a WithWaitingStatus handler) rather than the UI +// thread. See IGuiCommon.RefreshFromWorker. +func (self *RefreshHelper) RefreshFromWorker(options types.RefreshOptions) { + self.performRefresh(options, true) +} + +func (self *RefreshHelper) performRefresh(options types.RefreshOptions, calledFromWorker bool) { if options.Mode == types.ASYNC && options.Then != nil { panic("RefreshOptions.Then doesn't work with mode ASYNC") } @@ -101,6 +112,13 @@ func (self *RefreshHelper) Refresh(options types.RefreshOptions) { ) } + // f runs on the UI thread when the refresh was initiated there, and also for + // BLOCK_UI, which dispatches f onto the UI thread regardless of the caller. + // Only a SYNC/ASYNC refresh initiated from a worker runs f on that worker. + // This, not calledFromWorker alone, is what decides whether a scope capture + // runs inline or has to hop (see captureOnUIThread). + fRunsOnUIThread := options.Mode == types.BLOCK_UI || !calledFromWorker + f := func() { var scopeSet *set.Set[types.RefreshableView] if len(options.Scope) == 0 { @@ -188,8 +206,16 @@ func (self *RefreshHelper) Refresh(options types.RefreshOptions) { // whenever we change commits, we should update branches because the upstream/downstream // counts can change. Whenever we change branches we should also change commits // e.g. in the case of switching branches. + // Capture the commits refresh's model/context/mode inputs on the UI + // thread, before the git work is dispatched to a worker, so the + // worker computes from an immutable snapshot instead of reading + // state the UI thread concurrently mutates. + var capturedCommits capturedCommitState + self.captureOnUIThread(fRunsOnUIThread, options.Background, func() { + capturedCommits = self.captureCommitsState(options.CommitSelection) + }) refresh("commits and commit files", func() { - self.refreshCommitsAndCommitFiles(options.CommitSelection, options.Background) + self.refreshCommitsAndCommitFiles(capturedCommits, options.CommitSelection, options.Background) }) includeWorktreesWithBranches = scopeSet.Includes(types.WORKTREES) @@ -441,11 +467,49 @@ func (self *RefreshHelper) refreshReflogAndBranches(refreshWorktrees bool, branc return nil } -func (self *RefreshHelper) refreshCommitsAndCommitFiles(commitSelection types.CommitSelectionBehavior, background bool) { +// capturedCommitState holds everything the commits refresh reads from the +// model, contexts, and modes. It is gathered on the UI thread (see +// captureCommitsState) before the git work is dispatched to a worker, so the +// worker computes from an immutable snapshot rather than reading state the UI +// thread concurrently mutates. +type capturedCommitState struct { + selectionRange *localCommitSelectionRange + limitCommits bool + showWholeGitGraph bool + filterPath string + filterAuthor string + mainBranches *git_commands.MainBranches + hashPool *utils.StringPool + parentIsLocalCommits bool +} + +// captureCommitsState reads the commits refresh's model/context/mode inputs +// into an immutable snapshot. It must run on the UI thread. +func (self *RefreshHelper) captureCommitsState(commitSelection types.CommitSelectionBehavior) capturedCommitState { + var selectionRange *localCommitSelectionRange + if commitSelection == types.KeepCommitSelectionByHash { + selectedIdx, rangeStartIdx, rangeSelectMode := self.c.Contexts().LocalCommits.GetSelectionRangeAndMode() + selectionRange = captureLocalCommitSelectionRange(self.c.Model().Commits, selectedIdx, rangeStartIdx, rangeSelectMode) + } + + parentCtx := self.c.Contexts().CommitFiles.GetParentContext() + + return capturedCommitState{ + selectionRange: selectionRange, + limitCommits: self.c.Contexts().LocalCommits.GetLimitCommits(), + showWholeGitGraph: self.c.Contexts().LocalCommits.GetShowWholeGitGraph(), + filterPath: self.c.Modes().Filtering.GetPath(), + filterAuthor: self.c.Modes().Filtering.GetAuthor(), + mainBranches: self.c.Model().MainBranches, + hashPool: self.c.Model().HashPool, + parentIsLocalCommits: parentCtx != nil && parentCtx.GetKey() == context.LOCAL_COMMITS_CONTEXT_KEY, + } +} + +func (self *RefreshHelper) refreshCommitsAndCommitFiles(captured capturedCommitState, commitSelection types.CommitSelectionBehavior, background bool) { generation := self.c.State().GetRepoGeneration() - _ = self.refreshCommitsWithLimit(commitSelection, background) - ctx := self.c.Contexts().CommitFiles.GetParentContext() - if ctx != nil && ctx.GetKey() == context.LOCAL_COMMITS_CONTEXT_KEY { + _ = self.refreshCommitsWithLimit(captured, commitSelection, background) + if captured.parentIsLocalCommits { // This makes sense when we've e.g. just amended a commit, meaning we get a new commit hash at the same position. // However if we've just added a brand new commit, it pushes the list down by one and so we would end up // showing the contents of a different commit than the one we initially entered. @@ -497,28 +561,22 @@ func (self *RefreshHelper) determineCheckedOutRef() models.Ref { return nil } -func (self *RefreshHelper) refreshCommitsWithLimit(commitSelection types.CommitSelectionBehavior, background bool) error { +func (self *RefreshHelper) refreshCommitsWithLimit(captured capturedCommitState, commitSelection types.CommitSelectionBehavior, background bool) error { generation := self.c.State().GetRepoGeneration() - var selectionRange *localCommitSelectionRange - if commitSelection == types.KeepCommitSelectionByHash { - selectedIdx, rangeStartIdx, rangeSelectMode := self.c.Contexts().LocalCommits.GetSelectionRangeAndMode() - selectionRange = captureLocalCommitSelectionRange(self.c.Model().Commits, selectedIdx, rangeStartIdx, rangeSelectMode) - } - checkedOutRef := self.determineCheckedOutRef() refName, bisectInfo := self.refForLog() commits, err := self.c.Git().Loaders.CommitLoader.GetCommits( git_commands.GetCommitsOptions{ - Limit: self.c.Contexts().LocalCommits.GetLimitCommits(), - FilterPath: self.c.Modes().Filtering.GetPath(), - FilterAuthor: self.c.Modes().Filtering.GetAuthor(), + Limit: captured.limitCommits, + FilterPath: captured.filterPath, + FilterAuthor: captured.filterAuthor, IncludeRebaseCommits: true, RefName: refName, RefForPushedStatus: checkedOutRef, - All: self.c.Contexts().LocalCommits.GetShowWholeGitGraph(), - MainBranches: self.c.Model().MainBranches, - HashPool: self.c.Model().HashPool, + All: captured.showWholeGitGraph, + MainBranches: captured.mainBranches, + HashPool: captured.hashPool, }, ) if err != nil { @@ -545,10 +603,10 @@ func (self *RefreshHelper) refreshCommitsWithLimit(commitSelection types.CommitS scrollSelectionIntoView = true } case types.KeepCommitSelectionByHash: - if selectionRange != nil { - selectedIdx, rangeStartIdx, didMove, found := findLocalCommitSelectionRange(commits, selectionRange) + if captured.selectionRange != nil { + selectedIdx, rangeStartIdx, didMove, found := findLocalCommitSelectionRange(commits, captured.selectionRange) if found { - self.c.Contexts().LocalCommits.SetSelectionRangeAndMode(selectedIdx, rangeStartIdx, selectionRange.mode) + self.c.Contexts().LocalCommits.SetSelectionRangeAndMode(selectedIdx, rangeStartIdx, captured.selectionRange.mode) scrollSelectionIntoView = didMove } } @@ -898,6 +956,36 @@ func (self *RefreshHelper) onUIThread(background bool, f func() error) { } } +// captureOnUIThread runs fn on the UI thread and returns once it has run. fn +// reads the model/context/mode state a refresh scope needs into locals, so the +// worker that follows computes from an immutable snapshot instead of reading +// state the UI thread concurrently mutates. When the enclosing refresh function +// runs on the UI thread (fRunsOnUIThread is true) fn runs inline; when it runs +// on a worker, fn is dispatched to the UI thread and we block for it. +// +// The inline case matters for correctness as much as the hop: a SYNC or +// BLOCK_UI refresh parks the UI thread in a wg.Wait while its scope workers +// run, so a scope worker that tried to hop to the UI thread there would +// deadlock. Capturing before those workers are spawned — inline, on the UI +// thread — avoids that entirely. This is why BLOCK_UI (which always runs on the +// UI thread, even from a worker caller) captures inline rather than hopping. +func (self *RefreshHelper) captureOnUIThread(fRunsOnUIThread bool, background bool, fn func()) { + if fRunsOnUIThread { + fn() + return + } + + wrapped := func() error { + fn() + return nil + } + if background { + _ = self.c.GocuiGui().OnUIThreadAndWaitBackground(wrapped) + } else { + _ = self.c.GocuiGui().OnUIThreadAndWait(wrapped) + } +} + func (self *RefreshHelper) refreshStateFiles(background bool, submoduleConfigs []*models.SubmoduleConfig) error { fileTreeViewModel := self.c.Contexts().Files.FileTreeViewModel generation := self.c.State().GetRepoGeneration() diff --git a/pkg/gui/controllers/helpers/refs_helper.go b/pkg/gui/controllers/helpers/refs_helper.go index 8d5e8397c..70ff18593 100644 --- a/pkg/gui/controllers/helpers/refs_helper.go +++ b/pkg/gui/controllers/helpers/refs_helper.go @@ -55,7 +55,7 @@ func (self *RefsHelper) CheckoutRef(ref string, options types.CheckoutRefOptions if options.RefreshPullRequests { scope = append(scope, types.PULL_REQUESTS) } - self.c.Refresh(types.RefreshOptions{ + self.c.RefreshFromWorker(types.RefreshOptions{ Mode: types.BLOCK_UI, Scope: scope, BranchSelection: types.SelectCheckedOutBranch, @@ -204,7 +204,7 @@ func (self *RefsHelper) ResetToRef(ref string, strength string, envVars []string // loading a heap of commits is slow so we limit them whenever doing a reset self.c.Contexts().LocalCommits.SetLimitCommits(true) - self.c.Refresh(types.RefreshOptions{Scope: []types.RefreshableView{types.FILES, types.BRANCHES, types.REFLOG, types.COMMITS}, CommitSelection: types.KeepCommitSelectionIndex}) + self.c.RefreshFromWorker(types.RefreshOptions{Scope: []types.RefreshableView{types.FILES, types.BRANCHES, types.REFLOG, types.COMMITS}, CommitSelection: types.KeepCommitSelectionIndex}) return nil } @@ -523,7 +523,7 @@ func (self *RefsHelper) moveCommitsToNewBranchStackedOnCurrentBranch(newBranchNa } } - self.c.Refresh(types.RefreshOptions{ + self.c.RefreshFromWorker(types.RefreshOptions{ Mode: types.BLOCK_UI, BranchSelection: types.SelectCheckedOutBranch, CommitSelection: types.SelectHeadCommit, @@ -564,7 +564,7 @@ func (self *RefsHelper) moveCommitsToNewBranchOffOfMainBranch(newBranchName stri } } - self.c.Refresh(types.RefreshOptions{ + self.c.RefreshFromWorker(types.RefreshOptions{ Mode: types.BLOCK_UI, BranchSelection: types.SelectCheckedOutBranch, CommitSelection: types.SelectHeadCommit, diff --git a/pkg/gui/controllers/helpers/worktree_helper.go b/pkg/gui/controllers/helpers/worktree_helper.go index abfd1c0f6..980d810ae 100644 --- a/pkg/gui/controllers/helpers/worktree_helper.go +++ b/pkg/gui/controllers/helpers/worktree_helper.go @@ -163,7 +163,7 @@ func (self *WorktreeHelper) remove(worktree *models.Worktree, force bool, then f return then(task) } - self.c.Refresh(types.RefreshOptions{Mode: types.ASYNC, Scope: []types.RefreshableView{types.WORKTREES, types.BRANCHES, types.FILES}}) + self.c.RefreshFromWorker(types.RefreshOptions{Mode: types.ASYNC, Scope: []types.RefreshableView{types.WORKTREES, types.BRANCHES, types.FILES}}) return nil }) } @@ -181,7 +181,7 @@ func (self *WorktreeHelper) Detach(worktree *models.Worktree, then func(gocui.Ta return then(task) } - self.c.Refresh(types.RefreshOptions{Mode: types.ASYNC, Scope: []types.RefreshableView{types.WORKTREES, types.BRANCHES, types.FILES}}) + self.c.RefreshFromWorker(types.RefreshOptions{Mode: types.ASYNC, Scope: []types.RefreshableView{types.WORKTREES, types.BRANCHES, types.FILES}}) return nil }) } diff --git a/pkg/gui/controllers/local_commits_controller.go b/pkg/gui/controllers/local_commits_controller.go index 5150e8fba..04c7fc290 100644 --- a/pkg/gui/controllers/local_commits_controller.go +++ b/pkg/gui/controllers/local_commits_controller.go @@ -487,7 +487,7 @@ func (self *LocalCommitsController) handleReword(summary string, description str if err != nil { return err } - self.c.Refresh(types.RefreshOptions{Mode: types.ASYNC}) + self.c.RefreshFromWorker(types.RefreshOptions{Mode: types.ASYNC}) return nil }) } @@ -854,7 +854,7 @@ func (self *LocalCommitsController) resetAuthor(start, end int) error { return err } - self.c.Refresh(types.RefreshOptions{Mode: types.ASYNC}) + self.c.RefreshFromWorker(types.RefreshOptions{Mode: types.ASYNC}) return nil }) } @@ -870,7 +870,7 @@ func (self *LocalCommitsController) setAuthor(start, end int) error { return err } - self.c.Refresh(types.RefreshOptions{Mode: types.ASYNC}) + self.c.RefreshFromWorker(types.RefreshOptions{Mode: types.ASYNC}) return nil }) }, @@ -889,7 +889,7 @@ func (self *LocalCommitsController) addCoAuthor(start, end int) error { if err := self.c.Git().Rebase.AddCommitCoAuthor(self.c.Model().Commits, start, end, value); err != nil { return err } - self.c.Refresh(types.RefreshOptions{Mode: types.ASYNC}) + self.c.RefreshFromWorker(types.RefreshOptions{Mode: types.ASYNC}) return nil }) }, diff --git a/pkg/gui/controllers/remotes_controller.go b/pkg/gui/controllers/remotes_controller.go index 8bd19ad81..d4c838f7c 100644 --- a/pkg/gui/controllers/remotes_controller.go +++ b/pkg/gui/controllers/remotes_controller.go @@ -391,7 +391,7 @@ func (self *RemotesController) fetchAndCheckout(remote *models.Remote, branchNam } } } - self.c.Refresh(refreshOptions) + self.c.RefreshFromWorker(refreshOptions) return err }) } diff --git a/pkg/gui/controllers/sync_controller.go b/pkg/gui/controllers/sync_controller.go index 0f754eb49..fafd4e7dd 100644 --- a/pkg/gui/controllers/sync_controller.go +++ b/pkg/gui/controllers/sync_controller.go @@ -229,7 +229,7 @@ func (self *SyncController) pushAux(currentBranch *models.Branch, opts pushOpts) } return err } - self.c.Refresh(types.RefreshOptions{Mode: types.SYNC}) + self.c.RefreshFromWorker(types.RefreshOptions{Mode: types.SYNC}) return nil }) } diff --git a/pkg/gui/controllers/tags_controller.go b/pkg/gui/controllers/tags_controller.go index fe2c4e80f..2a59af5ec 100644 --- a/pkg/gui/controllers/tags_controller.go +++ b/pkg/gui/controllers/tags_controller.go @@ -168,7 +168,7 @@ func (self *TagsController) localDelete(tag *models.Tag) error { return self.c.WithWaitingStatus(self.c.Tr.DeletingStatus, func(gocui.Task) error { self.c.LogAction(self.c.Tr.Actions.DeleteLocalTag) err := self.c.Git().Tag.LocalDelete(tag.Name) - self.c.Refresh(types.RefreshOptions{Mode: types.ASYNC, Scope: []types.RefreshableView{types.COMMITS, types.TAGS}}) + self.c.RefreshFromWorker(types.RefreshOptions{Mode: types.ASYNC, Scope: []types.RefreshableView{types.COMMITS, types.TAGS}}) return err }) } @@ -210,7 +210,7 @@ func (self *TagsController) remoteDelete(tag *models.Tag) error { return err } self.c.Toast(self.c.Tr.RemoteTagDeletedMessage) - self.c.Refresh(types.RefreshOptions{Mode: types.SYNC, Scope: []types.RefreshableView{types.COMMITS, types.TAGS}}) + self.c.RefreshFromWorker(types.RefreshOptions{Mode: types.SYNC, Scope: []types.RefreshableView{types.COMMITS, types.TAGS}}) return nil }) }, @@ -264,7 +264,7 @@ func (self *TagsController) localAndRemoteDelete(tag *models.Tag) error { if err := self.c.Git().Tag.LocalDelete(tag.Name); err != nil { return err } - self.c.Refresh(types.RefreshOptions{Mode: types.SYNC, Scope: []types.RefreshableView{types.COMMITS, types.TAGS}}) + self.c.RefreshFromWorker(types.RefreshOptions{Mode: types.SYNC, Scope: []types.RefreshableView{types.COMMITS, types.TAGS}}) return nil }) }, diff --git a/pkg/gui/controllers/undo_controller.go b/pkg/gui/controllers/undo_controller.go index 775e871a4..0954d66b0 100644 --- a/pkg/gui/controllers/undo_controller.go +++ b/pkg/gui/controllers/undo_controller.go @@ -271,7 +271,7 @@ func (self *UndoController) hardResetWithAutoStash(commitHash string, options ha if err != nil { return err } - self.c.Refresh(types.RefreshOptions{}) + self.c.RefreshFromWorker(types.RefreshOptions{}) return nil }) } diff --git a/pkg/gui/gui_common.go b/pkg/gui/gui_common.go index d13120508..c8de7545d 100644 --- a/pkg/gui/gui_common.go +++ b/pkg/gui/gui_common.go @@ -30,6 +30,10 @@ func (self *guiCommon) Refresh(opts types.RefreshOptions) { self.gui.helpers.Refresh.Refresh(opts) } +func (self *guiCommon) RefreshFromWorker(opts types.RefreshOptions) { + self.gui.helpers.Refresh.RefreshFromWorker(opts) +} + func (self *guiCommon) PostRefreshUpdate(context types.Context) { self.gui.postRefreshUpdate(context) } diff --git a/pkg/gui/services/custom_commands/handler_creator.go b/pkg/gui/services/custom_commands/handler_creator.go index 1e321c2de..4eb762019 100644 --- a/pkg/gui/services/custom_commands/handler_creator.go +++ b/pkg/gui/services/custom_commands/handler_creator.go @@ -314,7 +314,7 @@ func (self *HandlerCreator) finalHandler(customCommand config.CustomCommand, ses } output, err := cmdObj.RunWithOutput() - self.c.Refresh(types.RefreshOptions{Mode: types.ASYNC}) + self.c.RefreshFromWorker(types.RefreshOptions{Mode: types.ASYNC}) if err != nil { if customCommand.After != nil && customCommand.After.CheckForConflicts { diff --git a/pkg/gui/types/common.go b/pkg/gui/types/common.go index 766a5a757..0f8e99ee8 100644 --- a/pkg/gui/types/common.go +++ b/pkg/gui/types/common.go @@ -30,6 +30,12 @@ type IGuiCommon interface { LogCommand(cmdStr string, isCommandLine bool) // we call this when we want to refetch some models and render the result. Internally calls PostRefreshUpdate Refresh(RefreshOptions) + // Like Refresh, but for callers running on a worker goroutine (e.g. inside + // a WithWaitingStatus handler) rather than the UI thread. The refresh + // captures the model/context state it needs on the UI thread before doing + // its git work; knowing which thread the caller is on lets it capture + // inline (UI thread) or hop across (worker) without racing or deadlocking. + RefreshFromWorker(RefreshOptions) // we call this when we've changed something in the view model but not the actual model, // e.g. expanding or collapsing a folder in a file view. Calling 'Refresh' in this // case would be overkill, although refresh will internally call 'PostRefreshUpdate'