diff --git a/pkg/gocui/gui.go b/pkg/gocui/gui.go index ee1995911..ff113a2dd 100644 --- a/pkg/gocui/gui.go +++ b/pkg/gocui/gui.go @@ -193,6 +193,12 @@ type Gui struct { taskManager *TaskManager + // The task of the event currently being processed on the main goroutine, if + // any. Only touched from the main goroutine (in processEvent). It's excluded + // from the Busy() check so that an event handler asking "is anything else + // busy?" doesn't count itself. + currentTask Task + lastHoverView *View } @@ -273,7 +279,15 @@ func NewGui(opts NewGuiOpts) (*Gui, error) { } func (g *Gui) NewTask() *TaskImpl { - return g.taskManager.NewTask() + return g.taskManager.NewTask(false) +} + +// Busy reports whether any foreground work is in flight, ignoring the event +// currently being processed on the main goroutine (see currentTask). Background +// routines (auto-fetch etc.) don't count. It's used to decide whether it's safe +// to switch repos. Must be called on the main goroutine. +func (g *Gui) Busy() bool { + return g.taskManager.hasBusyForegroundTaskExcept(g.currentTask) } // An idle listener listens for when the program is idle. This is useful for @@ -628,7 +642,18 @@ type userEvent struct { // never fire in practice; if it does, that's a signal to investigate, not // to grow the buffer reflexively. func (g *Gui) Update(f func(*Gui) error) { - task := g.NewTask() + g.update(f, false) +} + +// Like Update, but the enqueued work is a background routine (or triggered by +// one), so it doesn't count towards the program being busy for repo-switch +// safety. See TaskImpl.background. +func (g *Gui) UpdateBackground(f func(*Gui) error) { + g.update(f, true) +} + +func (g *Gui) update(f func(*Gui) error, background bool) { + task := g.taskManager.NewTask(background) select { case g.userEvents <- userEvent{f: f, task: task}: @@ -639,7 +664,16 @@ func (g *Gui) Update(f func(*Gui) error) { // Like Update, but signals that the callback only modifies content. func (g *Gui) UpdateContentOnly(f func(*Gui) error) { - task := g.NewTask() + g.updateContentOnly(f, false) +} + +// Like UpdateContentOnly, but for background work (see UpdateBackground). +func (g *Gui) UpdateContentOnlyBackground(f func(*Gui) error) { + g.updateContentOnly(f, true) +} + +func (g *Gui) updateContentOnly(f func(*Gui) error, background bool) { + task := g.taskManager.NewTask(background) g.userEvents <- userEvent{f: f, task: task, contentOnly: true} } @@ -650,7 +684,18 @@ func (g *Gui) UpdateContentOnly(f func(*Gui) error) { // background goroutines where you wouldn't want lazygit to be considered busy // (i.e. when you wouldn't want a loader to be shown to the user) func (g *Gui) OnWorker(f func(Task) error) { - task := g.NewTask() + g.onWorker(f, false) +} + +// Like OnWorker, but for a background routine (or work triggered by one), so it +// doesn't count towards the program being busy for repo-switch safety. See +// TaskImpl.background. +func (g *Gui) OnWorkerBackground(f func(Task) error) { + g.onWorker(f, true) +} + +func (g *Gui) onWorker(f func(Task) error, background bool) { + task := g.taskManager.NewTask(background) go func() { g.onWorkerAux(f, task) task.Done() @@ -758,17 +803,25 @@ func (g *Gui) handleError(err error) error { func (g *Gui) processEvent() error { contentOnly := false + // currentTask is the task of the event we're about to handle; recording it + // lets Busy() ignore it, so a handler asking "is anything else busy?" (the + // repo-switch guard does) doesn't count itself. Handlers of the remaining + // events drained below run with currentTask still set to this primary event; + // that's fine because the only Busy() callers are keybinding handlers, which + // are always the primary event here. select { case ev := <-g.gEvents: task := g.NewTask() - defer func() { task.Done() }() + g.currentTask = task + defer func() { g.currentTask = nil; task.Done() }() if err := g.handleError(g.handleEvent(&ev)); err != nil { return err } case ev := <-g.userEvents: contentOnly = ev.contentOnly - defer func() { ev.task.Done() }() + g.currentTask = ev.task + defer func() { g.currentTask = nil; ev.task.Done() }() if err := g.handleError(ev.f(g)); err != nil { return err diff --git a/pkg/gocui/task.go b/pkg/gocui/task.go index ace72f4a8..377781a4f 100644 --- a/pkg/gocui/task.go +++ b/pkg/gocui/task.go @@ -8,8 +8,9 @@ type Task interface { Done() Pause() Continue() - // not exporting because we don't need to + // not exporting these because we don't need to isBusy() bool + isBackground() bool } type TaskImpl struct { @@ -17,6 +18,13 @@ type TaskImpl struct { busy bool onDone func() withMutex func(func()) + // Background tasks don't count towards the program being "busy" for the + // purpose of deciding whether a repo switch is safe (see + // TaskManager.hasBusyForegroundTaskExcept). They're the ongoing background + // routines (auto-fetch, files refresh, external-change detection) and the + // refreshes they trigger, whose model writes are already guarded against a + // concurrent repo switch by the repo generation. + background bool } func (self *TaskImpl) Done() { @@ -39,6 +47,10 @@ func (self *TaskImpl) isBusy() bool { return self.busy } +func (self *TaskImpl) isBackground() bool { + return self.background +} + type TaskStatus int const ( @@ -73,6 +85,10 @@ func (self *FakeTask) isBusy() bool { return self.status == TaskStatusBusy } +func (self *FakeTask) isBackground() bool { + return false +} + func (self *FakeTask) Status() TaskStatus { return self.status } diff --git a/pkg/gocui/task_manager.go b/pkg/gocui/task_manager.go index e3c82b4d4..23ef0f77e 100644 --- a/pkg/gocui/task_manager.go +++ b/pkg/gocui/task_manager.go @@ -22,7 +22,7 @@ func newTaskManager() *TaskManager { } } -func (self *TaskManager) NewTask() *TaskImpl { +func (self *TaskManager) NewTask(background bool) *TaskImpl { self.mutex.Lock() defer self.mutex.Unlock() @@ -30,12 +30,34 @@ func (self *TaskManager) NewTask() *TaskImpl { taskId := self.nextId onDone := func() { self.delete(taskId) } - task := &TaskImpl{id: taskId, busy: true, onDone: onDone, withMutex: self.withMutex} + task := &TaskImpl{id: taskId, busy: true, background: background, onDone: onDone, withMutex: self.withMutex} self.tasks[taskId] = task return task } +// hasBusyForegroundTaskExcept reports whether any task other than `ignore` is +// currently busy and not a background task. It's used to decide whether a repo +// switch is safe: a foreground operation (or the refresh it triggers, or that +// refresh's follow-up callbacks) still in flight means the switch must wait, so +// it doesn't run against a repo that's about to be swapped out. +// +// `ignore` is the event currently being processed on the UI thread — the switch +// attempt itself — which is always busy and so must not count as a reason to +// refuse itself. +func (self *TaskManager) hasBusyForegroundTaskExcept(ignore Task) bool { + self.mutex.Lock() + defer self.mutex.Unlock() + + for _, task := range self.tasks { + if task != ignore && task.isBusy() && !task.isBackground() { + return true + } + } + + return false +} + func (self *TaskManager) addIdleListener(c chan struct{}) { self.idleListeners = append(self.idleListeners, c) } diff --git a/pkg/gocui/task_manager_test.go b/pkg/gocui/task_manager_test.go new file mode 100644 index 000000000..7fe706d7a --- /dev/null +++ b/pkg/gocui/task_manager_test.go @@ -0,0 +1,63 @@ +package gocui + +import ( + "testing" + + "github.com/stretchr/testify/assert" +) + +func TestTaskManagerHasBusyForegroundTaskExcept(t *testing.T) { + t.Run("no tasks", func(t *testing.T) { + tm := newTaskManager() + assert.False(t, tm.hasBusyForegroundTaskExcept(nil)) + }) + + t.Run("a busy foreground task counts", func(t *testing.T) { + tm := newTaskManager() + tm.NewTask(false) + assert.True(t, tm.hasBusyForegroundTaskExcept(nil)) + }) + + t.Run("a busy background task does not count", func(t *testing.T) { + tm := newTaskManager() + tm.NewTask(true) + assert.False(t, tm.hasBusyForegroundTaskExcept(nil)) + }) + + t.Run("a done foreground task does not count", func(t *testing.T) { + tm := newTaskManager() + task := tm.NewTask(false) + task.Done() + assert.False(t, tm.hasBusyForegroundTaskExcept(nil)) + }) + + t.Run("a paused foreground task does not count", func(t *testing.T) { + tm := newTaskManager() + task := tm.NewTask(false) + task.Pause() + assert.False(t, tm.hasBusyForegroundTaskExcept(nil)) + }) + + t.Run("the ignored task does not count", func(t *testing.T) { + tm := newTaskManager() + task := tm.NewTask(false) + assert.False(t, tm.hasBusyForegroundTaskExcept(task)) + }) + + t.Run("another foreground task counts even when one is ignored", func(t *testing.T) { + tm := newTaskManager() + ignored := tm.NewTask(false) + tm.NewTask(false) + assert.True(t, tm.hasBusyForegroundTaskExcept(ignored)) + }) + + t.Run("only a background task alongside the ignored current event", func(t *testing.T) { + // This is the repo-switch case: the switch is handled as the current + // event (ignored) while a background refresh is in flight; it must not + // be considered busy. + tm := newTaskManager() + current := tm.NewTask(false) + tm.NewTask(true) + assert.False(t, tm.hasBusyForegroundTaskExcept(current)) + }) +} diff --git a/pkg/gui/gui.go b/pkg/gui/gui.go index 834cf1e2d..5aa8beaec 100644 --- a/pkg/gui/gui.go +++ b/pkg/gui/gui.go @@ -1194,16 +1194,32 @@ func (gui *Gui) onUIThread(f func() error) { }) } +func (gui *Gui) onUIThreadBackground(f func() error) { + gui.g.UpdateBackground(func(*gocui.Gui) error { + return f() + }) +} + func (gui *Gui) onUIThreadContentOnly(f func() error) { gui.g.UpdateContentOnly(func(*gocui.Gui) error { return f() }) } +func (gui *Gui) onUIThreadContentOnlyBackground(f func() error) { + gui.g.UpdateContentOnlyBackground(func(*gocui.Gui) error { + return f() + }) +} + func (gui *Gui) onWorker(f func(gocui.Task) error) { gui.g.OnWorker(f) } +func (gui *Gui) onWorkerBackground(f func(gocui.Task) error) { + gui.g.OnWorkerBackground(f) +} + func (gui *Gui) getWindowDimensions(informationStr string, appStatus string) map[string]boxlayout.Dimensions { return gui.helpers.WindowArrangement.GetWindowDimensions(informationStr, appStatus) } diff --git a/pkg/gui/gui_common.go b/pkg/gui/gui_common.go index c74a99a05..d13120508 100644 --- a/pkg/gui/gui_common.go +++ b/pkg/gui/gui_common.go @@ -124,14 +124,26 @@ func (self *guiCommon) OnUIThread(f func() error) { self.gui.onUIThread(f) } +func (self *guiCommon) OnUIThreadBackground(f func() error) { + self.gui.onUIThreadBackground(f) +} + func (self *guiCommon) OnUIThreadContentOnly(f func() error) { self.gui.onUIThreadContentOnly(f) } +func (self *guiCommon) OnUIThreadContentOnlyBackground(f func() error) { + self.gui.onUIThreadContentOnlyBackground(f) +} + func (self *guiCommon) OnWorker(f func(gocui.Task) error) { self.gui.onWorker(f) } +func (self *guiCommon) OnWorkerBackground(f func(gocui.Task) error) { + self.gui.onWorkerBackground(f) +} + func (self *guiCommon) RenderToMainViews(opts types.RefreshMainOpts) { self.gui.refreshMainViews(opts) } diff --git a/pkg/gui/types/common.go b/pkg/gui/types/common.go index 2ce07f9c7..766a5a757 100644 --- a/pkg/gui/types/common.go +++ b/pkg/gui/types/common.go @@ -75,13 +75,22 @@ type IGuiCommon interface { // Only necessary to call if you're not already on the UI thread i.e. you're inside a goroutine. // All controller handlers are executed on the UI thread. OnUIThread(f func() error) + // Like OnUIThread, but for work triggered by a background routine, so it + // doesn't count towards lazygit being busy (see the *Background methods on + // gocui.Gui and repo-switch safety). + OnUIThreadBackground(f func() error) // Like OnUIThread, but signals that the callback only modifies view // content (e.g. spinner), allows the event loop to skip // the expensive layout recalculation when only content changed. OnUIThreadContentOnly(f func() error) + // Like OnUIThreadContentOnly, but for background work (see OnUIThreadBackground). + OnUIThreadContentOnlyBackground(f func() error) // Runs a function in a goroutine. Use this whenever you want to run a goroutine and keep track of the fact // that lazygit is still busy. See docs/dev/Busy.md OnWorker(f func(gocui.Task) error) + // Like OnWorker, but for a background routine (or work it triggers), so it + // doesn't count towards lazygit being busy (see OnUIThreadBackground). + OnWorkerBackground(f func(gocui.Task) error) // Function to call at the end of our 'layout' function which renders views // For example, you may want a view's line to be focused only after that view is // resized, if in accordion mode.