From a1561a5e6967cf8a7f93c2f787afed4c773a9d61 Mon Sep 17 00:00:00 2001 From: Stefan Haller Date: Fri, 17 Jul 2026 15:55:01 +0200 Subject: [PATCH] Render the app status in a single background render loop MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Each status used to start a spinner render loop of its own, running on a worker that inherited the foreground/background flavor of the status's owner, and exiting only once the entire status stack was empty. That shape had a real bug: a foreground operation's loop could be kept alive by someone else's status. Finish a quick operation with a waiting status while a background fetch's "Fetching..." status is still showing, and the operation's render loop — a foreground worker task — keeps ticking until the fetch ends. Busy() stays true for that whole time, so repo switching is refused even though nothing is in flight anymore; with a fetch hanging on a slow network, that means minutes. The shape was also wasteful: overlapping statuses were each drawn by their own loop (plus a duplicate whenever a task was paused and resumed while another status was showing), all redundantly redrawing the same top status. Replace the per-status loops with a single loop owned by the status stack as a whole: whoever shows the first status starts it, and it exits after drawing a final empty frame once the last status is removed. The claim/release methods on StatusManager keep the loop flag's transitions atomic with the stack under the one mutex, so a status added while the loop is about to exit starts a fresh loop instead of going unrendered. The loop always runs as a background task now: rendering issues no git commands, so it never needs to block repo switching, and a foreground operation's busy-ness is already carried by its own worker task. This is what fixes the bug above, and it retires the need to thread a foreground/background flag through the waiting-status helpers altogether. Co-Authored-By: Claude Fable 5 --- pkg/gui/background.go | 2 +- .../controllers/helpers/app_status_helper.go | 57 ++++++++++--------- pkg/gui/status/status_manager.go | 38 +++++++++++++ 3 files changed, 68 insertions(+), 29 deletions(-) diff --git a/pkg/gui/background.go b/pkg/gui/background.go index f9eff420b..1e2db853f 100644 --- a/pkg/gui/background.go +++ b/pkg/gui/background.go @@ -119,7 +119,7 @@ func (self *BackgroundRoutineMgr) startBackgroundFetch() { if self.gui.UserConfig().Gui.ShowBottomLine || firstTimeOrRetriggered { return self.gui.helpers.AppStatus.WithWaitingStatusImpl(self.gui.Tr.FetchingStatus, func(gocui.Task) error { return self.backgroundFetch() - }, nil, true) + }, nil) } return self.backgroundFetch() diff --git a/pkg/gui/controllers/helpers/app_status_helper.go b/pkg/gui/controllers/helpers/app_status_helper.go index 69daa7d7a..d0bb03395 100644 --- a/pkg/gui/controllers/helpers/app_status_helper.go +++ b/pkg/gui/controllers/helpers/app_status_helper.go @@ -34,12 +34,7 @@ func (self *AppStatusHelper) Toast(message string, kind types.ToastKind) { self.statusMgr().AddToastStatus(message, kind) - // Render the toast in the background: it's a transient notification, not - // lazygit driving an operation, so it must not count towards being busy — - // otherwise a toast (e.g. the "can't switch, operation in progress" one) - // would itself block a repo switch until it faded. A real operation showing - // a toast still keeps its own foreground task busy independently. - self.renderAppStatus(true) + self.renderAppStatus() } // A custom task for WithWaitingStatus calls; it wraps the original one and @@ -66,14 +61,15 @@ func (self appStatusHelperTask) Continue() { // WithWaitingStatus wraps a function and shows a waiting status while the function is still executing func (self *AppStatusHelper) WithWaitingStatus(message string, f func(gocui.Task) error) { self.c.OnWorker(func(task gocui.Task) error { - return self.WithWaitingStatusImpl(message, f, task, false) + return self.WithWaitingStatusImpl(message, f, task) }) } -// background reports whether this waiting status belongs to a background routine -// (the auto-fetch poller); when it does, the spinner it drives must not count -// towards lazygit being busy, or it'd block repo switches while a fetch runs. -func (self *AppStatusHelper) WithWaitingStatusImpl(message string, f func(gocui.Task) error, task gocui.Task, background bool) error { +// WithWaitingStatusImpl is WithWaitingStatus for callers that already run on a +// goroutine of their own (e.g. the auto-fetch poller) rather than wanting the +// work dispatched to a worker. task is used to hide the status while the task +// is paused; it may be nil for callers whose f ignores its task. +func (self *AppStatusHelper) WithWaitingStatusImpl(message string, f func(gocui.Task) error, task gocui.Task) error { // A waiting status means lazygit is driving a git operation itself (often // one that internally runs a rebase and continues it). Pause the background // routines for its duration so they don't refresh from an intermediate @@ -81,7 +77,7 @@ func (self *AppStatusHelper) WithWaitingStatusImpl(message string, f func(gocui. self.c.PauseBackgroundRefreshes(true) defer self.c.PauseBackgroundRefreshes(false) - return self.statusMgr().WithWaitingStatus(message, func() { self.renderAppStatus(background) }, func(waitingStatusHandle *status.WaitingStatusHandle) error { + return self.statusMgr().WithWaitingStatus(message, self.renderAppStatus, func(waitingStatusHandle *status.WaitingStatusHandle) error { return f(appStatusHelperTask{task, waitingStatusHandle}) }) } @@ -112,7 +108,7 @@ func (self *AppStatusHelper) WithWaitingStatusBlockingInput(message string, f fu self.modeHelper.SetSuppressRebasingMode(false) return self.c.GocuiGui().EndBlockingEvents() }) - return self.WithWaitingStatusImpl(message, f, task, false) + return self.WithWaitingStatusImpl(message, f, task) }) } @@ -125,33 +121,36 @@ func (self *AppStatusHelper) GetStatusString() string { return appStatus } -func (self *AppStatusHelper) renderAppStatus(background bool) { - // A background waiting status (auto-fetch) must not count towards lazygit - // being busy, so its spinner worker and per-frame UI updates go through the - // background variants. - onWorker := self.c.OnWorker - onUIThread := self.c.OnUIThread - onUIThreadContentOnly := self.c.OnUIThreadContentOnly - if background { - onWorker = self.c.OnWorkerBackground - onUIThread = self.c.OnUIThreadBackground - onUIThreadContentOnly = self.c.OnUIThreadContentOnlyBackground +// renderAppStatus ensures the render loop that keeps the app-status view up to +// date is running. There is one loop for the whole status stack, no matter how +// many statuses are showing: it draws whatever the top status currently is, +// and exits after drawing a final empty frame once the last status is removed. +// +// The loop always runs as a background task, regardless of what kind of +// operation owns a status: rendering runs no git commands, so it must never +// count towards lazygit being busy — otherwise it would block repo switching +// for as long as anything is showing (e.g. for the whole duration of a hung +// background fetch, or of a toast fading). A foreground operation's busy-ness +// is carried by its own worker task, not by the renderer. +func (self *AppStatusHelper) renderAppStatus() { + if !self.statusMgr().ClaimRenderLoop() { + return } - onWorker(func(_ gocui.Task) error { + self.c.OnWorkerBackground(func(_ gocui.Task) error { ticker := time.NewTicker(time.Millisecond * time.Duration(self.c.UserConfig().Gui.Spinner.Rate)) defer ticker.Stop() prevAppStatus := "" for range ticker.C { appStatus, color := self.statusMgr().GetStatusString(self.c.UserConfig()) - update := onUIThreadContentOnly + update := self.c.OnUIThreadContentOnlyBackground if utils.StringWidth(appStatus) != utils.StringWidth(prevAppStatus) { // Need a full layout whenever the width of the status string changes. This can't // happen during normal spinning because we validate that all spinner frames have // the same width, so typically this will only be triggered at the beginning and end // of a status, or if the status string changes midway for some reason. - update = onUIThread + update = self.c.OnUIThreadBackground } update(func() error { self.c.Views().AppStatus.FgColor = color @@ -160,7 +159,9 @@ func (self *AppStatusHelper) renderAppStatus(background bool) { }) prevAppStatus = appStatus - if appStatus == "" { + // Checked after rendering, so that the frame which clears the view + // has already been drawn when we exit. + if self.statusMgr().ReleaseRenderLoopIfEmpty() { break } } diff --git a/pkg/gui/status/status_manager.go b/pkg/gui/status/status_manager.go index 414568a69..35e1b7746 100644 --- a/pkg/gui/status/status_manager.go +++ b/pkg/gui/status/status_manager.go @@ -17,6 +17,11 @@ type StatusManager struct { statuses []appStatus nextId int mutex deadlock.Mutex + + // Whether a render loop is currently drawing the statuses. Guarded by + // mutex, so that claiming and releasing the loop stay atomic with the + // changes to statuses; see ClaimRenderLoop and ReleaseRenderLoopIfEmpty. + renderLoopRunning bool } // Can be used to manipulate a waiting status while it is running (e.g. pause @@ -90,6 +95,39 @@ func (self *StatusManager) HasStatus() bool { return len(self.statuses) > 0 } +// ClaimRenderLoop is called by whoever just added a status; it reports whether +// they must start the render loop. When it returns false, a loop is already +// running and will pick the new status up on its next tick. +func (self *StatusManager) ClaimRenderLoop() bool { + self.mutex.Lock() + defer self.mutex.Unlock() + + if self.renderLoopRunning { + return false + } + + self.renderLoopRunning = true + return true +} + +// ReleaseRenderLoopIfEmpty is called by the render loop after each frame it +// draws; a true result releases the loop's claim and tells it to exit, because +// there are no statuses left to draw. The emptiness check and the release are +// atomic with respect to ClaimRenderLoop, so a status added around this moment +// either sees the still-running loop or starts a fresh one — it can't end up +// unrendered. +func (self *StatusManager) ReleaseRenderLoopIfEmpty() bool { + self.mutex.Lock() + defer self.mutex.Unlock() + + if len(self.statuses) > 0 { + return false + } + + self.renderLoopRunning = false + return true +} + func (self *StatusManager) addStatus(message string, statusType string, kind types.ToastKind) int { self.mutex.Lock() defer self.mutex.Unlock()