From 58e121b9330836c3201f73f8246787ef67536345 Mon Sep 17 00:00:00 2001 From: Stefan Haller Date: Fri, 10 Jul 2026 16:48:42 +0200 Subject: [PATCH 1/2] Don't block the UI thread when triggering an immediate fetch on repo switch Switching repos triggers an immediate background fetch by sending on the goEvery retrigger channel. The send was blocking, but the goEvery loop only receives between callbacks: while a fetch is in flight, it waits for that fetch to finish before returning to its select. So a repo switch that landed while a fetch was in flight would stall the UI thread for the remainder of the fetch. Worse, since worker refreshes capture state on the UI thread with a blocking OnUIThreadAndWaitBackground call, the in-flight fetch's post-fetch refresh can itself be waiting for the UI thread, turning that stall into a deadlock cycle: UI thread: switchTo -> triggerImmediateFetch, blocking send goEvery loop: waiting for the in-flight fetch to finish fetch worker: PostFetchRefresh -> RefreshFromWorker, waiting for the UI thread Make the send non-blocking, and give the channel a buffer of one so that a trigger arriving while a fetch is in flight is latched rather than dropped; that fetch is fetching the previous repo, so we still need another one after it. The goEvery loop picks the trigger up as soon as it returns to its select, and concurrent triggers coalesce. Co-Authored-By: Claude Fable 5 --- pkg/gui/background.go | 18 ++++++++++++++++-- 1 file changed, 16 insertions(+), 2 deletions(-) diff --git a/pkg/gui/background.go b/pkg/gui/background.go index 8633f4624..8b5e4b8d4 100644 --- a/pkg/gui/background.go +++ b/pkg/gui/background.go @@ -190,7 +190,11 @@ func (self *BackgroundRoutineMgr) checkForExternalChanges() { // returns a channel that can be used to trigger the callback immediately func (self *BackgroundRoutineMgr) goEvery(interval time.Duration, stop chan struct{}, function func(bool) error) chan struct{} { done := make(chan struct{}) - retrigger := make(chan struct{}) + // Buffered so that a retrigger arriving while the callback is running is + // latched rather than lost: the loop below doesn't receive again until the + // callback has finished, and the callback (a fetch) may be for the wrong + // repo if the retrigger came from a repo switch. + retrigger := make(chan struct{}, 1) go utils.Safe(func() { ticker := time.NewTicker(interval) defer ticker.Stop() @@ -234,6 +238,16 @@ func (self *BackgroundRoutineMgr) backgroundFetch() (err error) { func (self *BackgroundRoutineMgr) triggerImmediateFetch() { if self.triggerFetch != nil { - self.triggerFetch <- struct{}{} + // This runs on the UI thread, which must never block waiting for a + // background routine; in particular, the goEvery loop only receives + // between callbacks, and an in-flight fetch can itself be waiting for + // the UI thread to perform its post-fetch refresh, so a blocking send + // here would deadlock. The channel has a buffer of one, so the trigger + // is latched even when the loop isn't currently receiving; if one is + // already pending, the two coalesce. + select { + case self.triggerFetch <- struct{}{}: + default: + } } } From 3a0ba6bf4d33ce355237497b560a19bfd4a855ba Mon Sep 17 00:00:00 2001 From: Stefan Haller Date: Fri, 10 Jul 2026 17:16:46 +0200 Subject: [PATCH 2/2] Fix data race on the triggerFetch field startBackgroundFetch assigned the field from its own goroutine, and only after the initial fetch had completed, while the UI thread reads it in triggerImmediateFetch on every repo switch, with no synchronization. Create the channel in startBackgroundRoutines instead, which runs on the UI thread before the fetch goroutine is spawned; everything the UI thread does afterwards is ordered after the write, so the read is race-free without any locking. To make this possible, goEvery now takes the retrigger channel as a parameter instead of creating and returning it; callers that have no use for a retrigger channel pass nil, and a nil channel in a select is simply never ready. As a side effect, a repo switch that happens before the fetch loop has started (during the intro popup or the initial fetch) now latches a trigger and causes an immediate fetch once the loop is running, where previously it was silently dropped. Co-Authored-By: Claude Fable 5 --- pkg/gui/background.go | 23 ++++++++++++----------- 1 file changed, 12 insertions(+), 11 deletions(-) diff --git a/pkg/gui/background.go b/pkg/gui/background.go index 8b5e4b8d4..f9eff420b 100644 --- a/pkg/gui/background.go +++ b/pkg/gui/background.go @@ -43,6 +43,11 @@ func (self *BackgroundRoutineMgr) startBackgroundRoutines() { if userConfig.Git.AutoFetch { fetchInterval := userConfig.Refresher.FetchInterval if fetchInterval > 0 { + // The channel must be created here, on the UI thread and before + // the fetch goroutine spawns, so that triggerImmediateFetch (also + // running on the UI thread) can read the field without racing the + // write. See triggerImmediateFetch for why it is buffered. + self.triggerFetch = make(chan struct{}, 1) go utils.Safe(self.startBackgroundFetch) } else { self.gui.c.Log.Errorf( @@ -74,7 +79,7 @@ func (self *BackgroundRoutineMgr) startBackgroundRoutines() { } if self.gui.Config.GetDebug() { - self.goEvery(time.Second*time.Duration(10), self.gui.stopChan, func(_ bool) error { + self.goEvery(time.Second*time.Duration(10), self.gui.stopChan, nil, func(_ bool) error { formatBytes := func(b uint64) string { const unit = 1000 if b < unit { @@ -125,14 +130,14 @@ func (self *BackgroundRoutineMgr) startBackgroundFetch() { _ = fetch(true) userConfig := self.gui.UserConfig() - self.triggerFetch = self.goEvery(userConfig.Refresher.FetchIntervalDuration(), self.gui.stopChan, fetch) + self.goEvery(userConfig.Refresher.FetchIntervalDuration(), self.gui.stopChan, self.triggerFetch, fetch) } func (self *BackgroundRoutineMgr) startBackgroundFilesRefresh() { self.gui.waitForIntro.Wait() userConfig := self.gui.UserConfig() - self.goEvery(userConfig.Refresher.RefreshIntervalDuration(), self.gui.stopChan, func(_ bool) error { + self.goEvery(userConfig.Refresher.RefreshIntervalDuration(), self.gui.stopChan, nil, func(_ bool) error { self.gui.c.RefreshFromWorker(types.RefreshOptions{Scope: []types.RefreshableView{types.FILES}, Background: true}) return nil }) @@ -151,6 +156,7 @@ func (self *BackgroundRoutineMgr) startBackgroundExternalChangeDetection() { self.goEvery( userConfig.Refresher.ExternalChangeCheckIntervalDuration(), self.gui.stopChan, + nil, func(_ bool) error { self.checkForExternalChanges() return nil @@ -187,14 +193,10 @@ func (self *BackgroundRoutineMgr) checkForExternalChanges() { self.gui.c.RefreshFromWorker(types.RefreshOptions{Background: true}) } -// returns a channel that can be used to trigger the callback immediately -func (self *BackgroundRoutineMgr) goEvery(interval time.Duration, stop chan struct{}, function func(bool) error) chan struct{} { +// Runs function every interval until stop is closed. A send on retrigger (if +// non-nil) runs the callback immediately and restarts the interval. +func (self *BackgroundRoutineMgr) goEvery(interval time.Duration, stop, retrigger chan struct{}, function func(bool) error) { done := make(chan struct{}) - // Buffered so that a retrigger arriving while the callback is running is - // latched rather than lost: the loop below doesn't receive again until the - // callback has finished, and the callback (a fetch) may be for the wrong - // repo if the retrigger came from a repo switch. - retrigger := make(chan struct{}, 1) go utils.Safe(func() { ticker := time.NewTicker(interval) defer ticker.Stop() @@ -227,7 +229,6 @@ func (self *BackgroundRoutineMgr) goEvery(interval time.Duration, stop chan stru } } }) - return retrigger } func (self *BackgroundRoutineMgr) backgroundFetch() (err error) {