run all terminal work on the UI goroutine and lock shared client state
Route network and audio callbacks through a bounded UI queue. Protocol handlers, the audio thread, and key handlers all drew to termbox widgets directly, which is a data race against the render loop. postUI is now the only path from a callback to a widget, work is dropped during shutdown, and the queue never blocks the caller. Guard the mutable client state with mutexes. Connection and transmission flags, the selected user, the muted channel set, and the audio stream pointer were each read and written from at least two goroutines. Lookups that walk the client's user and channel maps take the client lock and copy what they need. Snapshot the display string when a tree item is built. Tree items held live pointers and formatted themselves during rendering, so a user removed between rebuild and draw was dereferenced on the render path. Cancel reconnect retries on shutdown and release audio before reconnecting. Quitting during a retry left the goroutine sleeping until its timer expired, and a reconnect built a second OpenAL stream on top of the first. Cleanup is idempotent so repeated disconnect events are safe. Make UI shutdown idempotent and join the event poller. Close could be called more than once, and the polling goroutine could be left trying to deliver an event after Run had returned. Run now also reports a termbox initialization failure instead of returning nil. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
1 parent
a564286402
commit
872149c977
13 files changed
+957
-272
No files matched your search
+58
-10
@@ -3,7 +3,9 @@ package uiterm
|
||||
import (
|
||||
"errors"
|
||||
"strings"
|
||||
"sync"
|
||||
"sync/atomic"
|
||||
"time"
|
||||
|
||||
"github.com/nsf/termbox-go"
|
||||
)
|
||||
@@ -20,8 +22,10 @@ type UiManager interface {
|
||||
type Ui struct {
|
||||
Fg, Bg Attribute
|
||||
|
||||
close chan bool
|
||||
manager UiManager
|
||||
close chan struct{}
|
||||
closeOnce sync.Once
|
||||
events chan func()
|
||||
manager UiManager
|
||||
|
||||
drawCount int32
|
||||
elements map[string]*uiElement
|
||||
@@ -39,7 +43,8 @@ type uiElement struct {
|
||||
|
||||
func New(manager UiManager) *Ui {
|
||||
ui := &Ui{
|
||||
close: make(chan bool, 10),
|
||||
close: make(chan struct{}),
|
||||
events: make(chan func(), 256),
|
||||
elements: make(map[string]*uiElement),
|
||||
manager: manager,
|
||||
keyListeners: make(map[Key][]KeyListener),
|
||||
@@ -48,9 +53,24 @@ func New(manager UiManager) *Ui {
|
||||
return ui
|
||||
}
|
||||
|
||||
// Close is safe to call repeatedly and never blocks a caller.
|
||||
func (ui *Ui) Close() {
|
||||
if termbox.IsInit {
|
||||
ui.close <- true
|
||||
ui.closeOnce.Do(func() { close(ui.close) })
|
||||
}
|
||||
|
||||
// Post schedules UI work on Run's owning goroutine. It is deliberately
|
||||
// bounded: network callbacks must not block behind slow terminal rendering.
|
||||
func (ui *Ui) Post(fn func()) bool {
|
||||
if fn == nil {
|
||||
return true
|
||||
}
|
||||
select {
|
||||
case <-ui.close:
|
||||
return false
|
||||
case ui.events <- fn:
|
||||
return true
|
||||
default:
|
||||
return false
|
||||
}
|
||||
}
|
||||
|
||||
@@ -97,15 +117,37 @@ func (ui *Ui) Run(cmds chan string) error {
|
||||
return nil
|
||||
}
|
||||
if err := termbox.Init(); err != nil {
|
||||
return nil
|
||||
return err
|
||||
}
|
||||
defer termbox.Close()
|
||||
termbox.SetInputMode(termbox.InputAlt)
|
||||
|
||||
events := make(chan termbox.Event)
|
||||
// Closing termbox wakes PollEvent. Keep delivery cancellable so the polling
|
||||
// goroutine cannot become stranded trying to send after Run returns.
|
||||
events := make(chan termbox.Event, 1)
|
||||
pollDone := make(chan struct{})
|
||||
go func() {
|
||||
defer close(pollDone)
|
||||
for {
|
||||
events <- termbox.PollEvent()
|
||||
event := termbox.PollEvent()
|
||||
select {
|
||||
case <-ui.close:
|
||||
return
|
||||
default:
|
||||
}
|
||||
select {
|
||||
case events <- event:
|
||||
case <-ui.close:
|
||||
return
|
||||
}
|
||||
}
|
||||
}()
|
||||
defer func() {
|
||||
termbox.Close()
|
||||
// Some termbox backends do not wake PollEvent promptly on Close. A fatal
|
||||
// startup failure must print its stderr error instead of hanging here.
|
||||
select {
|
||||
case <-pollDone:
|
||||
case <-time.After(100 * time.Millisecond):
|
||||
}
|
||||
}()
|
||||
|
||||
@@ -119,7 +161,13 @@ func (ui *Ui) Run(cmds chan string) error {
|
||||
select {
|
||||
case <-ui.close:
|
||||
return nil
|
||||
case cmd := <-cmds:
|
||||
case fn := <-ui.events:
|
||||
fn()
|
||||
case cmd, ok := <-cmds:
|
||||
if !ok {
|
||||
cmds = nil
|
||||
continue
|
||||
}
|
||||
ui.onCommandEvent(cmd)
|
||||
case event := <-events:
|
||||
switch event.Type {
|
||||
|
||||
@@ -0,0 +1,45 @@
|
||||
package uiterm
|
||||
|
||||
import "testing"
|
||||
|
||||
// Regression: Close sent to a bounded channel and could block or enqueue
|
||||
// duplicate shutdowns when called more than once.
|
||||
// Regression: network callbacks modified terminal state directly. Post gives
|
||||
// them a bounded handoff to the UI-owning Run goroutine instead of blocking.
|
||||
func TestPostIsBoundedAndRejectsClosedUI(t *testing.T) {
|
||||
ui := New(nil)
|
||||
for i := 0; i < cap(ui.events); i++ {
|
||||
if !ui.Post(func() {}) {
|
||||
t.Fatal("queue filled too early")
|
||||
}
|
||||
}
|
||||
if ui.Post(func() {}) {
|
||||
t.Fatal("Post accepted work past queue capacity")
|
||||
}
|
||||
ui.Close()
|
||||
if ui.Post(func() {}) {
|
||||
t.Fatal("Post accepted work after close")
|
||||
}
|
||||
}
|
||||
|
||||
func TestCloseIsNonblockingAndIdempotent(t *testing.T) {
|
||||
ui := New(nil)
|
||||
ui.Close()
|
||||
ui.Close()
|
||||
select {
|
||||
case <-ui.close:
|
||||
default:
|
||||
t.Fatal("Close did not signal shutdown")
|
||||
}
|
||||
}
|
||||
|
||||
func TestSafeRuneRemovesTerminalControlCharacters(t *testing.T) {
|
||||
for _, r := range []rune{'\x1b', '\x7f', '\u202e'} {
|
||||
if got := safeRune(r); got != ' ' {
|
||||
t.Errorf("safeRune(%U) = %U, want space", r, got)
|
||||
}
|
||||
}
|
||||
if got := safeRune('A'); got != 'A' {
|
||||
t.Fatalf("safeRune altered printable text: %U", got)
|
||||
}
|
||||
}
|
||||
Reference in new issue
Block a user