Fix a deadlock between task.Done() and the integration test's idle wait

Running the integration tests in a loop under the race detector
eventually hung in demo/bisect. The goroutine dump shows the cycle: a
background worker's task.Done() held the task manager's mutex while
blocking on the unbuffered idle-listener channel send, and the test
runner goroutine — the only reader of that channel — was itself blocked
in NewTask on that same mutex, on its way to enqueueing a caption
render (SetCaption -> Render -> OnUIThread). Neither side could
proceed: the notification couldn't be delivered until the test
goroutine got the mutex, and the mutex couldn't be released until the
notification was delivered.

The root problem is that the busy-to-idle notification is a blocking
rendezvous performed while holding the mutex, so it needs the waiter's
cooperation at a moment where the waiter may legitimately need the
mutex first.

Make the notification fire-and-forget instead: WaitUntilIdle waits on a
condition variable and re-checks "is any task busy?" under the mutex,
and the busy-to-idle transition broadcasts, which never blocks. Waiting
is now level-triggered rather than edge-triggered, which is also more
robust: a wait can no longer be satisfied by a stale idle transition
produced by an unrelated background task, because the predicate is
evaluated against the current state. This relies on the previous commit
having made replayed input events carry their task from submission;
without that, the wait could return in the window where an event is in
flight but not yet picked up by the main loop.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

pkg/gocui/gui.go

@@ -326,11 +326,11 @@ 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
-// integration tests which can wait for the program to be idle before taking
-// the next step in the test.
-func (g *Gui) AddIdleListener(c chan struct{}) {
-	g.taskManager.addIdleListener(c)
+// WaitUntilIdle blocks until the program is idle (no busy tasks). This is
+// useful for integration tests which want to wait for the program to finish
+// processing before taking the next step in the test.
+func (g *Gui) WaitUntilIdle() {
+	g.taskManager.WaitUntilIdle()
 }
 
 // Close finalizes the library. It should be called after a successful

pkg/gocui/task_manager.go

@@ -6,20 +6,23 @@ import "sync"
 // the main goroutine or a worker goroutine). Used by integration tests
 // to wait until the program is idle before progressing.
 type TaskManager struct {
-	// each of these listeners will be notified when the program goes from busy to idle
-	idleListeners []chan struct{}
-	tasks         map[int]Task
+	tasks map[int]Task
 	// auto-incrementing id for new tasks
 	nextId int
 
 	mutex sync.Mutex
+	// signalled whenever the program transitions from busy to idle; used by
+	// WaitUntilIdle
+	idleCond *sync.Cond
 }
 
 func newTaskManager() *TaskManager {
-	return &TaskManager{
-		tasks:         make(map[int]Task),
-		idleListeners: []chan struct{}{},
+	self := &TaskManager{
+		tasks: make(map[int]Task),
 	}
+	self.idleCond = sync.NewCond(&self.mutex)
+
+	return self
 }
 
 func (self *TaskManager) NewTask(background bool) *TaskImpl {
@@ -58,8 +61,26 @@ func (self *TaskManager) hasBusyForegroundTaskExcept(ignore Task) bool {
 	return false
 }
 
-func (self *TaskManager) addIdleListener(c chan struct{}) {
-	self.idleListeners = append(self.idleListeners, c)
+// WaitUntilIdle blocks until no task is busy. Integration tests use it to wait
+// for the program to finish processing before taking the next step.
+func (self *TaskManager) WaitUntilIdle() {
+	self.mutex.Lock()
+	defer self.mutex.Unlock()
+
+	for self.hasBusyTask() {
+		self.idleCond.Wait()
+	}
+}
+
+// caller must hold self.mutex
+func (self *TaskManager) hasBusyTask() bool {
+	for _, task := range self.tasks {
+		if task.isBusy() {
+			return true
+		}
+	}
+
+	return false
 }
 
 func (self *TaskManager) withMutex(f func()) {
@@ -68,17 +89,12 @@ func (self *TaskManager) withMutex(f func()) {
 
 	f()
 
-	// Check if all tasks are done
-	for _, task := range self.tasks {
-		if task.isBusy() {
-			return
-		}
-	}
-
-	// If we get here, all tasks are done, so
-	// notify listeners that the program is idle
-	for _, listener := range self.idleListeners {
-		listener <- struct{}{}
+	// Wake up any goroutine blocked in WaitUntilIdle. This must not block on
+	// the waiter (we hold the mutex, and the waiter may itself be trying to
+	// acquire it, e.g. by creating a task, before it next waits) — which is
+	// exactly what Broadcast guarantees.
+	if !self.hasBusyTask() {
+		self.idleCond.Broadcast()
 	}
 }
 

pkg/gocui/task_manager_test.go

@@ -2,6 +2,7 @@ package gocui
 
 import (
 	"testing"
+	"time"
 
 	"github.com/stretchr/testify/assert"
 )
@@ -61,3 +62,68 @@ func TestTaskManagerHasBusyForegroundTaskExcept(t *testing.T) {
 		assert.False(t, tm.hasBusyForegroundTaskExcept(current))
 	})
 }
+
+func TestTaskManagerWaitUntilIdle(t *testing.T) {
+	// returnsWithin reports whether f returns within the given duration.
+	returnsWithin := func(d time.Duration, f func()) bool {
+		done := make(chan struct{})
+		go func() {
+			f()
+			close(done)
+		}()
+		select {
+		case <-done:
+			return true
+		case <-time.After(d):
+			return false
+		}
+	}
+
+	t.Run("returns immediately when no task was ever created", func(t *testing.T) {
+		tm := newTaskManager()
+		assert.True(t, returnsWithin(time.Second, tm.WaitUntilIdle))
+	})
+
+	t.Run("blocks while a task is busy", func(t *testing.T) {
+		tm := newTaskManager()
+		tm.NewTask(false)
+		assert.False(t, returnsWithin(50*time.Millisecond, tm.WaitUntilIdle))
+	})
+
+	t.Run("wakes up when the last busy task completes", func(t *testing.T) {
+		tm := newTaskManager()
+		task := tm.NewTask(false)
+		go func() {
+			time.Sleep(10 * time.Millisecond)
+			task.Done()
+		}()
+		assert.True(t, returnsWithin(time.Second, tm.WaitUntilIdle))
+	})
+
+	t.Run("a paused task counts as idle", func(t *testing.T) {
+		tm := newTaskManager()
+		task := tm.NewTask(false)
+		task.Pause()
+		assert.True(t, returnsWithin(time.Second, tm.WaitUntilIdle))
+	})
+
+	t.Run("a task completing while nobody waits must not block", func(t *testing.T) {
+		// This is the deadlock case: the waiter (the integration-test runner)
+		// is between waits, and itself needs the task manager's mutex (it
+		// creates a task whenever it enqueues work) before it waits again. The
+		// idle notification must neither block the completing task while it
+		// holds the mutex, nor get lost.
+		tm := newTaskManager()
+		assert.True(t, returnsWithin(time.Second, func() {
+			// the program goes idle with nobody waiting...
+			tm.NewTask(true).Done()
+
+			// ...and creating and completing more tasks afterwards must still
+			// be possible
+			task := tm.NewTask(false)
+			tm.NewTask(false).Done()
+			task.Done()
+		}))
+		assert.True(t, returnsWithin(time.Second, tm.WaitUntilIdle))
+	})
+}

pkg/gui/gui_driver.go

@@ -17,10 +17,9 @@ import (
 // this gives our integration test a way of interacting with the gui for sending keypresses
 // and reading state.
 type GuiDriver struct {
-	gui        *Gui
-	isIdleChan chan struct{}
-	toastChan  chan string
-	headless   bool
+	gui       *Gui
+	toastChan chan string
+	headless  bool
 }
 
 var _ integrationTypes.GuiDriver = &GuiDriver{}
@@ -79,7 +78,7 @@ func (self *GuiDriver) PretendMergeOrRebaseStartedInLazygit() {
 
 // wait until lazygit is idle (i.e. all processing is done) before continuing
 func (self *GuiDriver) waitTillIdle() {
-	<-self.isIdleChan
+	self.gui.g.WaitUntilIdle()
 }
 
 func (self *GuiDriver) CheckAllToastsAcknowledged() {

pkg/gui/test_mode.go

@@ -23,12 +23,8 @@ func (gui *Gui) handleTestMode() {
 	}
 
 	if test != nil {
-		isIdleChan := make(chan struct{})
-
-		gui.c.GocuiGui().AddIdleListener(isIdleChan)
-
 		waitUntilIdle := func() {
-			<-isIdleChan
+			gui.c.GocuiGui().WaitUntilIdle()
 		}
 
 		go func() {
@@ -38,7 +34,7 @@ func (gui *Gui) handleTestMode() {
 			gui.PopupHandler.(*popup.PopupHandler).SetToastFunc(
 				func(message string, kind types.ToastKind) { toastChan <- message })
 
-			test.Run(&GuiDriver{gui: gui, isIdleChan: isIdleChan, toastChan: toastChan, headless: Headless()})
+			test.Run(&GuiDriver{gui: gui, toastChan: toastChan, headless: Headless()})
 
 			gui.g.Update(func(*gocui.Gui) error {
 				return gocui.ErrQuit