diff options
| author | Robin Jarry <robin@jarry.cc> | 2023-05-17 16:13:43 +0200 |
|---|---|---|
| committer | Robin Jarry <robin@jarry.cc> | 2023-05-20 22:09:43 +0200 |
| commit | 916bca33ea6cf2530117071fdd9b7b15e00e2f29 (patch) | |
| tree | 3d3a523e6af3fd330da17a114d49db148c45c5fc /widgets | |
| parent | 02099cc6ea29e8595f43abe5b6a475edc92e24e0 (diff) | |
| download | aerc-916bca33ea6cf2530117071fdd9b7b15e00e2f29.tar.gz | |
ui: fix deadlocks in message channel
There are several ways the ui message channel can fill up leading to
deadlocks.
1) Invalidate() changes the value of uiState to DIRTY. The following
call sequence:
QueueRedraw()
Invalidate()
QueueRedraw()
Leads to multiple nil messages being queued in the message channel
whereas one could assume that the second QueueRedraw() would do
nothing. This is caused by the tri-state nature of uiState.
2) We use the same channel to convey state change, keyboard events and
redraw requests. Since a keyboard event almost always triggers
a redraw, we end up trying to append a redraw message in the same
goroutine that reads from the channel. This triggers a deadlock when
there are more than 50 pending messages.
Solve the issue by using multiple channels, one per type of message that
needs to be sent to the main ui thread.
Remove QueueRedraw() and merge its functionality in Invalidate(). Only
use a DIRTY/CLEAN state to determine if something needs to be queued in
the redraw channel.
Use a channel for quitting instead of an atomic. Restructure some code
functions to have a cleaner API.
Use a for loop in the main thread and select from all channels.
Signed-off-by: Robin Jarry <robin@jarry.cc>
Tested-by: Koni Marti <koni.marti@gmail.com>
Tested-by: Maarten van Gompel <proycon@anaproy.nl>
Diffstat (limited to 'widgets')
| -rw-r--r-- | widgets/aerc.go | 3 | ||||
| -rw-r--r-- | widgets/spinner.go | 2 | ||||
| -rw-r--r-- | widgets/terminal.go | 6 |
3 files changed, 4 insertions, 7 deletions
diff --git a/widgets/aerc.go b/widgets/aerc.go index 9f88ecb6..85ce686b 100644 --- a/widgets/aerc.go +++ b/widgets/aerc.go @@ -732,7 +732,6 @@ func (aerc *Aerc) Mailto(addr *url.URL) error { } defer ui.Invalidate() - defer ui.QueueRedraw() composer, err := NewComposer(aerc, acct, acct.AccountConfig(), acct.Worker(), template, h, nil) @@ -775,7 +774,6 @@ func (aerc *Aerc) Mbox(source string) error { acctConf.CopyTo = "Sent" defer ui.Invalidate() - defer ui.QueueRedraw() mboxView, err := NewAccountView(aerc, &acctConf, aerc, nil) if err != nil { @@ -788,7 +786,6 @@ func (aerc *Aerc) Mbox(source string) error { } func (aerc *Aerc) Command(args []string) error { - defer ui.QueueRedraw() defer ui.Invalidate() return aerc.cmd(args, nil, nil) } diff --git a/widgets/spinner.go b/widgets/spinner.go index abbe1ae4..63eaf11b 100644 --- a/widgets/spinner.go +++ b/widgets/spinner.go @@ -49,7 +49,7 @@ func (s *Spinner) Start() { return case <-time.After(s.interval): atomic.AddInt64(&s.frame, 1) - ui.QueueRedraw() + ui.Invalidate() } } }() diff --git a/widgets/terminal.go b/widgets/terminal.go index 9c2123cc..cf0888ee 100644 --- a/widgets/terminal.go +++ b/widgets/terminal.go @@ -58,7 +58,7 @@ func (term *Terminal) closeErr(err error) { term.ctx.HideCursor() } term.closed = true - ui.QueueRedraw() + ui.Invalidate() } func (term *Terminal) Destroy() { @@ -165,7 +165,7 @@ func (term *Terminal) HandleEvent(ev tcell.Event) { switch ev := ev.(type) { case *tcellterm.EventRedraw: if term.visible { - ui.QueueRedraw() + ui.Invalidate() } case *tcellterm.EventTitle: if term.OnTitle != nil { @@ -173,7 +173,7 @@ func (term *Terminal) HandleEvent(ev tcell.Event) { } case *tcellterm.EventClosed: term.Close() - ui.QueueRedraw() + ui.Invalidate() } } |