Make the user-event queue unbounded

Update and friends enqueued onto a fixed 256-slot channel with a
non-blocking send that panicked when the channel was full. That guard
was firing in real use:

 - Toggling a directory of several hundred files into a custom patch
   (reliably): the operation runs on a worker behind a waiting status,
   whose spinner enqueues a content-only render on every tick, and over
   the long operation these outrun the UI loop and overflow the buffer.
 - Editing the config in an editor that suspends lazygit: the editor
   subprocess runs on the UI thread, so the loop drains nothing for the
   whole editing session, and the full refresh fired on resume fans out
   across every scope at once — a burst of updates that overflows before
   the just-resumed loop catches up.
 - Any time the UI thread blocks for a long time, the periodic refreshes
   keep enqueuing and eventually overflow.

The 256-slot buffer was chosen deliberately, with the panic as a
"should never happen" guard, to preserve two properties: FIFO ordering
of same-goroutine Update calls (an earlier goroutine-per-Update design
reordered them), and no self-deadlock (a blocking send from the UI
thread would block against the loop that drains it). But a fixed
channel can only offer those by crashing on overflow.

Replace it with an unbounded, order-preserving queue: a mutex-guarded
slice plus a buffered(1) doorbell channel that wakes the main loop's
select. Enqueuing appends and rings the doorbell; the loop drains the
slice to empty on each wake. This keeps FIFO order and never blocks the
caller, so there is no self-deadlock and no overflow to panic on — under
a stall the queue just grows and then drains.

This also removes an inconsistency: updateContentOnly did a plain
blocking send while update panicked, so the two paths disagreed on what
happened when the queue was full. Both now share the same enqueue.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
Stefan Haller 2026-07-10 09:35:24 +02:00
parent 9f51f044fa
commit 49eefbcf37
3 changed files with 171 additions and 30 deletions

View file

@ -39,15 +39,15 @@ func setupViews(t *testing.T, g *Gui) (*View, *View) {
return status, main
}
// pushContentOnly pushes a content-only event directly to the channel
// (synchronous, deterministic — unlike Update which spawns a goroutine).
// pushContentOnly enqueues a content-only event directly, letting the test
// control the contentOnly flag (which Update/UpdateContentOnly hard-code).
func pushContentOnly(g *Gui, f func(*Gui) error) {
g.userEvents <- userEvent{f: f, task: g.NewTask(), contentOnly: true}
g.userEvents.enqueue(userEvent{f: f, task: g.NewTask(), contentOnly: true})
}
// pushRegular pushes a regular event directly to the channel.
// pushRegular enqueues a regular (non-content-only) event directly.
func pushRegular(g *Gui, f func(*Gui) error) {
g.userEvents <- userEvent{f: f, task: g.NewTask(), contentOnly: false}
g.userEvents.enqueue(userEvent{f: f, task: g.NewTask(), contentOnly: false})
}
func TestFlushContentOnly_SkipsUntaintedViews(t *testing.T) {

View file

@ -131,7 +131,7 @@ type Gui struct {
viewMouseBindings []*ViewMouseBinding
lastClick *clickInfo
gEvents chan GocuiEvent
userEvents chan userEvent
userEvents *userEventQueue
views []*View
currentView *View
managers []Manager
@ -238,12 +238,7 @@ func NewGui(opts NewGuiOpts) (*Gui, error) {
g.stop = make(chan struct{})
g.gEvents = make(chan GocuiEvent, 20)
// Update does a non-blocking send and panics on a full channel rather than
// blocking (which would deadlock the UI goroutine against itself) or
// silently reordering. The buffer is sized well above the peak occupancy we
// see in practice, so the panic stays unreachable in normal use; if it ever
// fires, that's a real anomaly to investigate, not a cue to grow the buffer.
g.userEvents = make(chan userEvent, 256)
g.userEvents = newUserEventQueue()
g.taskManager = newTaskManager()
if opts.PlayRecording {
@ -618,29 +613,83 @@ type userEvent struct {
contentOnly bool
}
// Update enqueues f on the user-events channel for the UI loop to run on its
// next iteration. Multiple Update calls from the same goroutine arrive in
// source order via the channel's FIFO. The send is non-blocking — if the
// channel is full we panic rather than block or silently reorder, since a
// blocked send from the UI goroutine would deadlock against itself and
// silently switching to inline execution would break the ordering guarantee
// callers rely on. The buffer is sized generously enough that this should
// 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()
// userEventQueue is an unbounded, order-preserving FIFO of work enqueued by
// Update and friends for the main loop to run.
//
// It's unbounded (rather than a fixed-size channel) because producers must
// never block or lose work. Update can be called from the UI goroutine itself,
// where a blocking send would deadlock against the loop that drains the queue;
// and it can be called from arbitrary worker goroutines that may enqueue faster
// than the loop drains. That happens while the loop is stalled — suspended for
// a subprocess (the editor runs on the UI thread), or hung in a long handler —
// and also when a long-running worker operation emits a steady stream of
// updates that outpaces the loop (e.g. the waiting-status spinner ticks while a
// large directory is toggled into a custom patch). A fixed channel forces a
// choice between blocking (deadlock), dropping or reordering, and panicking on
// overflow; an unbounded queue avoids all three while preserving FIFO order.
//
// enqueue appends under the mutex and rings the doorbell; the main loop selects
// on the doorbell to wake, then drains the slice to empty. The doorbell is
// buffered(1) and rung with a non-blocking send, so it's a coalescing "work
// pending" flag rather than a per-event signal: a burst of appends leaves at
// most one token, and the loop drains everything the token represents on a
// single wake. A token left over after a drain (because the drain happened to
// empty the slice after the ring) just causes one harmless empty wake.
type userEventQueue struct {
mutex sync.Mutex
events []userEvent
doorbell chan struct{}
}
func newUserEventQueue() *userEventQueue {
return &userEventQueue{doorbell: make(chan struct{}, 1)}
}
// enqueue appends an event and wakes the main loop. It never blocks.
func (q *userEventQueue) enqueue(ev userEvent) {
q.mutex.Lock()
q.events = append(q.events, ev)
q.mutex.Unlock()
select {
case g.userEvents <- userEvent{f: f, task: task}:
case q.doorbell <- struct{}{}:
default:
panic("gocui: userEvents channel full; refusing to block or reorder")
}
}
// dequeue pops the oldest event, reporting false when the queue is empty.
func (q *userEventQueue) dequeue() (userEvent, bool) {
q.mutex.Lock()
defer q.mutex.Unlock()
if len(q.events) == 0 {
return userEvent{}, false
}
ev := q.events[0]
if len(q.events) == 1 {
// Release the backing array whenever the queue drains, so a one-off
// burst doesn't pin its peak size for the rest of the session.
q.events = nil
} else {
q.events[0] = userEvent{}
q.events = q.events[1:]
}
return ev, true
}
// Update enqueues f for the UI loop to run on its next iteration. Multiple
// Update calls from the same goroutine arrive in source order (the queue is
// FIFO). The enqueue never blocks and never drops work; see userEventQueue for
// why the queue is unbounded.
func (g *Gui) Update(f func(*Gui) error) {
task := g.NewTask()
g.userEvents.enqueue(userEvent{f: f, task: task})
}
// Like Update, but signals that the callback only modifies content.
func (g *Gui) UpdateContentOnly(f func(*Gui) error) {
task := g.NewTask()
g.userEvents <- userEvent{f: f, task: task, contentOnly: true}
g.userEvents.enqueue(userEvent{f: f, task: task, contentOnly: true})
}
// Calls a function in a goroutine. Handles panics gracefully and tracks
@ -766,7 +815,14 @@ func (g *Gui) processEvent() error {
if err := g.handleError(g.handleEvent(&ev)); err != nil {
return err
}
case ev := <-g.userEvents:
case <-g.userEvents.doorbell:
ev, ok := g.userEvents.dequeue()
if !ok {
// A leftover doorbell token whose events were already drained by a
// previous iteration's processRemainingEvents: nothing to run and
// nothing new to render.
return nil
}
contentOnly = ev.contentOnly
defer func() { ev.task.Done() }()
@ -798,15 +854,20 @@ func (g *Gui) processRemainingEvents() (bool, error) {
if err := g.handleError(g.handleEvent(&ev)); err != nil {
return false, err
}
case ev := <-g.userEvents:
default:
// No gui event is pending; drain a queued user event instead.
// gui events take priority so input stays responsive, but they're
// bounded (buffer of 20), so this can't starve the user-event queue.
ev, ok := g.userEvents.dequeue()
if !ok {
return contentOnly, nil
}
contentOnly = ev.contentOnly && contentOnly
err := g.handleError(ev.f(g))
ev.task.Done()
if err != nil {
return false, err
}
default:
return contentOnly, nil
}
}
}

View file

@ -0,0 +1,80 @@
package gocui
import (
"sync"
"testing"
"github.com/stretchr/testify/assert"
)
// Enqueuing far more events than the old fixed 256-slot buffer, without the
// main loop draining them, used to panic ("userEvents channel full"). It must
// not: producers can legitimately burst faster than a stalled UI loop drains
// (e.g. one command-log entry per git command when adding a large directory to
// a custom patch, or any producer while the loop is blocked in a subprocess).
// The events must also stay in FIFO order.
func TestUpdateIsUnboundedAndPreservesOrder(t *testing.T) {
g := newTestGui(t)
const n = 1000
var got []int
for i := range n {
g.Update(func(*Gui) error {
got = append(got, i)
return nil
})
}
// Drain the whole queue the way the main loop's inner drain does.
_, err := g.processRemainingEvents()
assert.NoError(t, err)
want := make([]int, n)
for i := range want {
want[i] = i
}
assert.Equal(t, want, got)
}
// Concurrent producers must be able to enqueue safely (run under -race). Only
// same-goroutine order is guaranteed, so we check that every event is delivered
// exactly once and that each producer's own events stay in order.
func TestUpdateConcurrentProducers(t *testing.T) {
g := newTestGui(t)
const producers = 8
const perProducer = 500
type item struct{ producer, seq int }
var got []item
var wg sync.WaitGroup
for p := range producers {
wg.Add(1)
go func() {
defer wg.Done()
for seq := range perProducer {
g.Update(func(*Gui) error {
got = append(got, item{p, seq})
return nil
})
}
}()
}
// Update is a synchronous, non-blocking enqueue, so once every producer has
// returned, every event is in the queue and a single drain sees them all.
wg.Wait()
_, err := g.processRemainingEvents()
assert.NoError(t, err)
assert.Len(t, got, producers*perProducer)
lastSeq := make([]int, producers)
for p := range lastSeq {
lastSeq[p] = -1
}
for _, it := range got {
assert.Equal(t, lastSeq[it.producer]+1, it.seq, "producer %d events out of order", it.producer)
lastSeq[it.producer] = it.seq
}
}