aerc: use single event loop

Combine tcell events with WorkerMessages to better synchronize state
with IO and UI. Remove Tick loop for rendering. Use events to trigger
renders.

Signed-off-by: Tim Culverhouse <tim@timculverhouse.com>
Acked-by: Robin Jarry <robin@jarry.cc>

CHANGELOG.md

@@ -26,6 +26,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/).
 - Running the same command multiple times only adds one entry to the command
   history.
 - Embedded terminal backend (libvterm was replaced by a pure go implementation).
+- Use event driven loop instead of Tick based
 
 ### Fixed
 
@@ -35,6 +36,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/).
   can be disabled by setting `outgoing-cred-cmd-cache=false` in
   `accounts.conf`.
 - Mouse support for embedded editors when `mouse-enabled=true`.
+- Numerous race conditions related to event handling order
 
 ## [0.12.0](https://git.sr.ht/~rjarry/aerc/refs/0.12.0) - 2022-09-01
 

aerc.go

@@ -9,9 +9,9 @@ import (
 	"runtime"
 	"sort"
 	"strings"
-	"time"
 
 	"git.sr.ht/~sircmpwn/getopt"
+	"github.com/gdamore/tcell/v2"
 	"github.com/mattn/go-isatty"
 	"github.com/xo/terminfo"
 
@@ -28,6 +28,7 @@ import (
 	libui "git.sr.ht/~rjarry/aerc/lib/ui"
 	"git.sr.ht/~rjarry/aerc/logging"
 	"git.sr.ht/~rjarry/aerc/widgets"
+	"git.sr.ht/~rjarry/aerc/worker/types"
 )
 
 func getCommands(selected libui.Drawable) []*commands.Commands {
@@ -241,15 +242,18 @@ func main() {
 		setWindowTitle()
 	}
 
-	go ui.ProcessEvents()
+	ui.ChannelEvents()
-	for !ui.ShouldExit() {
+	for event := range libui.MsgChannel {
-		for aerc.Tick() {
+		switch event := event.(type) {
-			// Continue updating our internal state
+		case tcell.Event:
+			ui.HandleEvent(event)
+		case types.WorkerMessage:
+			aerc.HandleMessage(event)
 		}
-		if !ui.Render() {
+		if ui.ShouldExit() {
-			// ~60 FPS
+			break
-			time.Sleep(16 * time.Millisecond)
 		}
+		ui.Render()
 	}
 	err = aerc.CloseBackends()
 	if err != nil {

lib/msgstore.go

@@ -7,6 +7,7 @@ import (
 
 	"git.sr.ht/~rjarry/aerc/lib/marker"
 	"git.sr.ht/~rjarry/aerc/lib/sort"
+	"git.sr.ht/~rjarry/aerc/lib/ui"
 	"git.sr.ht/~rjarry/aerc/logging"
 	"git.sr.ht/~rjarry/aerc/models"
 	"git.sr.ht/~rjarry/aerc/worker/types"
@@ -394,6 +395,7 @@ func (store *MessageStore) runThreadBuilder() {
 	}
 	store.threadBuilderDebounce = time.AfterFunc(store.threadBuilderDelay, func() {
 		store.runThreadBuilderNow()
+		ui.QueueRedraw()
 	})
 }
 

lib/ui/textinput.go

@@ -314,6 +314,7 @@ func (ti *TextInput) showCompletions() {
 	ti.completions, ti.prefix = ti.tabcomplete(ti.StringLeft())
 	ti.completeIndex = -1
 	ti.Invalidate()
+	QueueRedraw()
 }
 
 func (ti *TextInput) OnChange(onChange func(ti *TextInput)) {

lib/ui/ui.go

@@ -3,7 +3,6 @@ package ui
 import (
 	"sync/atomic"
 
-	"git.sr.ht/~rjarry/aerc/logging"
 	"github.com/gdamore/tcell/v2"
 )
 
@@ -108,21 +107,24 @@ func (state *UI) EnableMouse() {
 	state.screen.EnableMouse()
 }
 
-func (state *UI) ProcessEvents() {
+func (state *UI) ChannelEvents() {
-	defer logging.PanicHandler()
+	go func() {
+		for {
+			MsgChannel <- state.screen.PollEvent()
+		}
+	}()
+}
 
-	for !state.ShouldExit() {
+func (state *UI) HandleEvent(event tcell.Event) {
-		event := state.screen.PollEvent()
+	if event, ok := event.(*tcell.EventResize); ok {
-		if event, ok := event.(*tcell.EventResize); ok {
+		state.screen.Clear()
-			state.screen.Clear()
+		width, height := event.Size()
-			width, height := event.Size()
+		state.ctx = NewContext(width, height, state.screen, state.onPopover)
-			state.ctx = NewContext(width, height, state.screen, state.onPopover)
+		state.Content.Invalidate()
-			state.Content.Invalidate()
+	}
-		}
+	// if we have a popover, and it can handle the event, it does so
-		// if we have a popover, and it can handle the event, it does so
+	if state.popover == nil || !state.popover.Event(event) {
-		if state.popover == nil || !state.popover.Event(event) {
+		// otherwise, we send the event to the main content
-			// otherwise, we send the event to the main content
+		state.Content.Event(event)
-			state.Content.Event(event)
-		}
 	}
 }

widgets/account.go

@@ -74,7 +74,7 @@ func NewAccountView(aerc *Aerc, conf *config.AercConfig, acct *config.AccountCon
 		{Strategy: ui.SIZE_WEIGHT, Size: ui.Const(1)},
 	})
 
-	worker, err := worker.NewWorker(acct.Source)
+	worker, err := worker.NewWorker(acct.Source, acct.Name)
 	if err != nil {
 		host.SetError(fmt.Sprintf("%s: %s", acct.Name, err))
 		logging.Errorf("%s: %v", acct.Name, err)
@@ -110,20 +110,6 @@ func NewAccountView(aerc *Aerc, conf *config.AercConfig, acct *config.AccountCon
 	return view, nil
 }
 
-func (acct *AccountView) Tick() bool {
-	if acct.worker == nil {
-		return false
-	}
-	select {
-	case msg := <-acct.worker.Messages:
-		msg = acct.worker.ProcessMessage(msg)
-		acct.onMessage(msg)
-		return true
-	default:
-		return false
-	}
-}
-
 func (acct *AccountView) SetStatus(setters ...statusline.SetStateFunc) {
 	for _, fn := range setters {
 		fn(acct.state, acct.SelectedDirectory())
@@ -236,6 +222,7 @@ func (acct *AccountView) isSelected() bool {
 }
 
 func (acct *AccountView) onMessage(msg types.WorkerMessage) {
+	msg = acct.worker.ProcessMessage(msg)
 	switch msg := msg.(type) {
 	case *types.Done:
 		switch msg.InResponseTo().(type) {

widgets/aerc.go

@@ -20,6 +20,7 @@ import (
 	"git.sr.ht/~rjarry/aerc/lib/ui"
 	"git.sr.ht/~rjarry/aerc/logging"
 	"git.sr.ht/~rjarry/aerc/models"
+	"git.sr.ht/~rjarry/aerc/worker/types"
 )
 
 type Aerc struct {
@@ -145,26 +146,10 @@ func (aerc *Aerc) Beep() {
 	}
 }
 
-func (aerc *Aerc) Tick() bool {
+func (aerc *Aerc) HandleMessage(msg types.WorkerMessage) {
-	more := false
+	if acct, ok := aerc.accounts[msg.Account()]; ok {
-	for _, acct := range aerc.accounts {
+		acct.onMessage(msg)
-		more = acct.Tick() || more
 	}
-
-	if len(aerc.prompts.Children()) > 0 {
-		more = true
-		previous := aerc.focused
-		prompt := aerc.prompts.Pop().(*ExLine)
-		prompt.finish = func() {
-			aerc.statusbar.Pop()
-			aerc.focus(previous)
-		}
-
-		aerc.statusbar.Push(prompt)
-		aerc.focus(prompt)
-	}
-
-	return more
 }
 
 func (aerc *Aerc) OnInvalidate(onInvalidate func(d ui.Drawable)) {
@@ -182,6 +167,17 @@ func (aerc *Aerc) Focus(focus bool) {
 }
 
 func (aerc *Aerc) Draw(ctx *ui.Context) {
+	if len(aerc.prompts.Children()) > 0 {
+		previous := aerc.focused
+		prompt := aerc.prompts.Pop().(*ExLine)
+		prompt.finish = func() {
+			aerc.statusbar.Pop()
+			aerc.focus(previous)
+		}
+
+		aerc.statusbar.Push(prompt)
+		aerc.focus(prompt)
+	}
 	aerc.grid.Draw(ctx)
 	if aerc.dialog != nil {
 		if w, h := ctx.Width(), ctx.Height(); w > 8 && h > 4 {

widgets/spinner.go

@@ -49,6 +49,7 @@ func (s *Spinner) Start() {
 			case <-time.After(200 * time.Millisecond):
 				atomic.AddInt64(&s.frame, 1)
 				s.Invalidate()
+				ui.QueueRedraw()
 			}
 		}
 	}()

widgets/terminal.go

@@ -169,6 +169,7 @@ func (term *Terminal) HandleEvent(ev tcell.Event) bool {
 		}
 	case *tcellterm.EventClosed:
 		term.Close(nil)
+		ui.QueueRedraw()
 	}
 	return false
 }

worker/types/messages.go

@@ -12,11 +12,14 @@ type WorkerMessage interface {
 	InResponseTo() WorkerMessage
 	getId() int64
 	setId(id int64)
+	Account() string
+	setAccount(string)
 }
 
 type Message struct {
 	inResponseTo WorkerMessage
 	id           int64
+	acct         string
 }
 
 func RespondTo(msg WorkerMessage) Message {
@@ -37,6 +40,14 @@ func (m *Message) setId(id int64) {
 	m.id = id
 }
 
+func (m *Message) Account() string {
+	return m.acct
+}
+
+func (m *Message) setAccount(name string) {
+	m.acct = name
+}
+
 // Meta-messages
 
 type Done struct {

worker/types/worker.go

@@ -5,6 +5,7 @@ import (
 	"sync"
 	"sync/atomic"
 
+	"git.sr.ht/~rjarry/aerc/lib/ui"
 	"git.sr.ht/~rjarry/aerc/logging"
 	"git.sr.ht/~rjarry/aerc/models"
 )
@@ -16,9 +17,9 @@ type Backend interface {
 }
 
 type Worker struct {
-	Backend  Backend
+	Backend Backend
-	Actions  chan WorkerMessage
+	Actions chan WorkerMessage
-	Messages chan WorkerMessage
+	Name    string
 
 	actionCallbacks  map[int64]func(msg WorkerMessage)
 	messageCallbacks map[int64]func(msg WorkerMessage)
@@ -28,10 +29,10 @@ type Worker struct {
 	sync.Mutex
 }
 
-func NewWorker() *Worker {
+func NewWorker(name string) *Worker {
 	return &Worker{
 		Actions:          make(chan WorkerMessage),
-		Messages:         make(chan WorkerMessage, 50),
+		Name:             name,
 		actionCallbacks:  make(map[int64]func(msg WorkerMessage)),
 		messageCallbacks: make(map[int64]func(msg WorkerMessage)),
 		actionQueue:      list.New(),
@@ -103,13 +104,14 @@ func (worker *Worker) PostMessage(msg WorkerMessage,
 	cb func(msg WorkerMessage),
 ) {
 	worker.setId(msg)
+	msg.setAccount(worker.Name)
 
 	if resp := msg.InResponseTo(); resp != nil {
 		logging.Debugf("PostMessage %T:%T", msg, resp)
 	} else {
 		logging.Debugf("PostMessage %T", msg)
 	}
-	worker.Messages <- msg
+	ui.MsgChannel <- msg
 
 	if cb != nil {
 		worker.Lock()

worker/worker.go

@@ -9,12 +9,12 @@ import (
 )
 
 // Guesses the appropriate worker type based on the given source string
-func NewWorker(source string) (*types.Worker, error) {
+func NewWorker(source string, name string) (*types.Worker, error) {
 	u, err := url.Parse(source)
 	if err != nil {
 		return nil, err
 	}
-	worker := types.NewWorker()
+	worker := types.NewWorker(name)
 	scheme := u.Scheme
 	if strings.ContainsRune(scheme, '+') {
 		scheme = scheme[:strings.IndexRune(scheme, '+')]