Don't flush the screen while suspended

When suspending with ctrl+z, the suspend keybinding handler disengages
the screen and then sends SIGSTOP to the process group, so the UI
thread freezes at the return from kill(2) with the handler's follow-up
flush still pending. When fg continues the process, that pending flush
races the SIGCONT handler's Resume. If the flush wins, Show() draws
against the disengaged screen, whose cell buffer tcell has released to
0x0 while its width/height still hold the old size; drawCell() then
reports width 0 for the out-of-range cell, the draw loop's
'x += width - 1' never advances, and the UI thread spins forever while
holding the tcell screen lock. Resume in turn blocks forever on that
lock, so the screen never re-engages and no input is ever read again:
the hard stall of #5309, only recoverable by killing the process.

Guard both flush paths with the suspended flag. For the flag to
guarantee that the screen is engaged whenever it is false, Resume must
clear it only after re-engaging (it used to clear it before); Suspend
already sets it before disengaging. This also covers the pre-existing
unsynchronized suspended check in draw(), which is subsumed by the
guards and can go.

The regression test cannot use the demonstrate-then-fix pattern: on
unfixed code the flush goroutine spins holding the screen lock, which
deadlocks any subsequent screen call including the test cleanup's
Close().
This commit is contained in:
Stefan Haller 2026-07-17 13:58:32 +02:00
parent 3ed6ce8f67
commit d887a41ad2
2 changed files with 81 additions and 6 deletions

View file

@ -1470,6 +1470,11 @@ func (g *Gui) drawListFooter(v *View, fgColor, bgColor Attribute) error {
// flush updates the gui, re-drawing frames and buffers.
func (g *Gui) flush() error {
// The screen must not be touched while suspended (see Suspend).
if g.isSuspended() {
return nil
}
// pretty sure we don't need this, but keeping it here in case we get weird visual artifacts
// g.clear(g.FgColor, g.BgColor)
@ -1502,6 +1507,11 @@ func (g *Gui) flush() error {
// actually-changed cells are emitted to the terminal.
// Will also redraw any views that overlap tainted views
func (g *Gui) flushContentOnly(views []*View) error {
// The screen must not be touched while suspended (see Suspend).
if g.isSuspended() {
return nil
}
for _, v := range viewsToRedrawContentOnly(views) {
if err := g.draw(v); err != nil {
return err
@ -1555,10 +1565,6 @@ func (g *Gui) ForceFlushViewsContentOnly(views []*View) error {
// draw manages the cursor and calls the draw function of a view.
func (g *Gui) draw(v *View) error {
if g.suspended {
return nil
}
if !v.Visible || v.y1 < v.y0 || v.x1 < v.x0 {
return nil
}
@ -1930,6 +1936,14 @@ func (g *Gui) onFocus(ev *GocuiEvent) error {
return nil
}
// While g.suspended is true, nothing must be drawn to the screen: tcell
// releases the screen's cell buffer when disengaging, and drawing to a
// disengaged screen spins forever inside tcell while holding the screen lock,
// which then blocks Resume (and with it all further input) forever. For the
// flag to guarantee that, it must only ever be false while the screen is
// engaged: Suspend sets it before disengaging, and Resume clears it only
// after re-engaging.
func (g *Gui) Suspend() error {
g.suspendedMutex.Lock()
defer g.suspendedMutex.Unlock()
@ -1940,7 +1954,12 @@ func (g *Gui) Suspend() error {
g.suspended = true
return g.screen.Suspend()
if err := g.screen.Suspend(); err != nil {
g.suspended = false
return err
}
return nil
}
func (g *Gui) Resume() error {
@ -1951,9 +1970,20 @@ func (g *Gui) Resume() error {
return errors.New("Cannot resume because we are not suspended")
}
if err := g.screen.Resume(); err != nil {
return err
}
g.suspended = false
return g.screen.Resume()
return nil
}
func (g *Gui) isSuspended() bool {
g.suspendedMutex.Lock()
defer g.suspendedMutex.Unlock()
return g.suspended
}
// matchView returns if the keybinding matches the current view (and the view's context)

View file

@ -7,6 +7,51 @@ import (
"github.com/stretchr/testify/assert"
)
// A flush while suspended must return without touching the screen: tcell
// releases the screen's cell buffer when disengaging, and drawing to a
// disengaged screen spins forever inside tcell while holding the screen lock,
// blocking the resume triggered by fg (#5309). The flush runs in a goroutine
// so that a regression fails the test instead of hanging the suite.
func TestFlushIsNoOpWhileSuspended(t *testing.T) {
tests := []struct {
name string
flush func(g *Gui) error
}{
{"flush", func(g *Gui) error { return g.flush() }},
{"flushContentOnly", func(g *Gui) error { return g.flushContentOnly(g.views) }},
}
for _, tc := range tests {
t.Run(tc.name, func(t *testing.T) {
// Deliberately not newTestGui: its cleanup closes the screen,
// which would deadlock on the screen lock if a regression makes
// the flush below spin.
g, err := NewGui(NewGuiOpts{
OutputMode: OutputNormal,
Headless: true,
Width: 80,
Height: 24,
})
assert.NoError(t, err)
assert.NoError(t, g.Suspend())
flushReturned := make(chan error, 1)
go func() { flushReturned <- tc.flush(g) }()
select {
case err := <-flushReturned:
assert.NoError(t, err)
case <-time.After(time.Second):
t.Fatal("flush touched the suspended screen and got stuck")
}
assert.NoError(t, g.Resume())
g.Close()
})
}
}
func TestResumeSchedulesRedraw(t *testing.T) {
g := newTestGui(t)