mirror of
https://github.com/jesseduffield/lazygit.git
synced 2026-08-24 10:13:41 -05:00
Reset the view origin for a new task on the UI thread
When a task renders different content to a view (a new task key), the view's scroll origin is reset to the top via onNewKey. That ran on the task's own goroutine, racing the UI thread, which reads the origin (OriginY) while laying out and drawing the view -- the single largest source of view-render data races. Give ViewBufferManager a bounce primitive (onUIThread) that runs a function on the UI thread and waits for it, and reset the origin through it. This is the first use of the primitive; subsequent commits route the rest of the task's view mutations through it too, so that the view is only ever touched on the UI thread. It runs as background work (OnUIThreadAndWaitBackground) so rendering doesn't count towards the app being busy, matching how the render's gocui task is already created. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Opus 4.8
parent
303372d917
commit
99c1bcbf23
@@ -156,6 +156,9 @@ func (gui *Gui) getManager(view *gocui.View) *tasks.ViewBufferManager {
|
||||
// otherwise make the switch that handler triggers refuse itself.
|
||||
return gui.c.GocuiGui().NewBackgroundTask()
|
||||
},
|
||||
// Rendering is background work too (see above), so the view mutations
|
||||
// it bounces onto the UI thread mustn't count towards being busy.
|
||||
gui.g.OnUIThreadAndWaitBackground,
|
||||
)
|
||||
gui.viewBufferManagerMap[view.Name()] = manager
|
||||
}
|
||||
|
||||
+24
-6
@@ -74,6 +74,12 @@ type ViewBufferManager struct {
|
||||
// whereas the tasks in this file are about rendering content to a view.
|
||||
newGocuiTask func() gocui.Task
|
||||
|
||||
// Runs f on the UI thread and blocks until it has completed. All mutations
|
||||
// of the view happen through this, so that the view is only ever touched on
|
||||
// the UI thread (where it is also laid out and drawn), never on the task's
|
||||
// own goroutine.
|
||||
onUIThread func(f func() error) error
|
||||
|
||||
// if the user flicks through a heap of items, with each one
|
||||
// spawning a process to render something to the main view,
|
||||
// it can slow things down quite a bit. In these situations we
|
||||
@@ -110,6 +116,7 @@ func NewViewBufferManager(
|
||||
onEndOfInput func(),
|
||||
onNewKey func(),
|
||||
newGocuiTask func() gocui.Task,
|
||||
onUIThread func(f func() error) error,
|
||||
) *ViewBufferManager {
|
||||
return &ViewBufferManager{
|
||||
Log: log,
|
||||
@@ -120,6 +127,7 @@ func NewViewBufferManager(
|
||||
readLines: nil,
|
||||
onNewKey: onNewKey,
|
||||
newGocuiTask: newGocuiTask,
|
||||
onUIThread: onUIThread,
|
||||
}
|
||||
}
|
||||
|
||||
@@ -460,21 +468,31 @@ func (self *ViewBufferManager) NewTask(f func(TaskOpts) error, key string) error
|
||||
self.taskIDMutex.Lock()
|
||||
|
||||
// Bail out before touching shared view state if a newer task has
|
||||
// already been queued: if we ran onNewKey here we'd reset the view
|
||||
// for a task that's about to exit, potentially wiping output the
|
||||
// winning task has already written.
|
||||
// already been queued: if we reset the view here we'd do it for a task
|
||||
// that's about to exit, potentially wiping output the winning task has
|
||||
// already written.
|
||||
if taskID < self.newTaskID {
|
||||
self.taskIDMutex.Unlock()
|
||||
return
|
||||
}
|
||||
|
||||
if self.GetTaskKey() != key && self.onNewKey != nil {
|
||||
self.onNewKey()
|
||||
}
|
||||
resetOrigin := self.GetTaskKey() != key && self.onNewKey != nil
|
||||
self.taskKey = key
|
||||
|
||||
self.taskIDMutex.Unlock()
|
||||
|
||||
if resetOrigin {
|
||||
// onNewKey resets the view's scroll origin, which is view state the
|
||||
// UI thread reads while laying out and drawing, so do it there. This
|
||||
// must happen after releasing taskIDMutex: it blocks until the UI
|
||||
// thread runs it, and a NewTask call on the UI thread takes
|
||||
// taskIDMutex, so holding it here would deadlock.
|
||||
_ = self.onUIThread(func() error {
|
||||
self.onNewKey()
|
||||
return nil
|
||||
})
|
||||
}
|
||||
|
||||
self.waitingMutex.Lock()
|
||||
|
||||
// Re-check staleness after acquiring waitingMutex: a newer task
|
||||
|
||||
@@ -39,6 +39,8 @@ func TestNewCmdTaskInstantStop(t *testing.T) {
|
||||
onEndOfInput,
|
||||
onNewKey,
|
||||
newTask,
|
||||
// no UI thread in the test; run the view mutations inline
|
||||
func(f func() error) error { return f() },
|
||||
)
|
||||
|
||||
stop := make(chan struct{})
|
||||
@@ -104,6 +106,8 @@ func TestNewCmdTask(t *testing.T) {
|
||||
onEndOfInput,
|
||||
onNewKey,
|
||||
newTask,
|
||||
// no UI thread in the test; run the view mutations inline
|
||||
func(f func() error) error { return f() },
|
||||
)
|
||||
|
||||
stop := make(chan struct{})
|
||||
@@ -237,6 +241,8 @@ func TestNewCmdTaskRefresh(t *testing.T) {
|
||||
func() {},
|
||||
func() {},
|
||||
newTask,
|
||||
// no UI thread in the test; run the view mutations inline
|
||||
func(f func() error) error { return f() },
|
||||
)
|
||||
|
||||
stop := make(chan struct{})
|
||||
|
||||
Reference in New Issue
Block a user