aerc

Working clone of aerc-mail.org
git clone git://git.danielmoch.com/aerc.git
Log | Files | Refs | README | LICENSE

commit 5685a176747711d0330f1251ad297b13e0621a37
parent 9ef2a57b51601cc0e18824bbfe8026891da1a38d
Author: Simon Ser <contact@emersion.fr>
Date:   Sat, 27 Apr 2019 16:47:59 +0000

lib/ui: introduce Invalidatable

Many Drawable implementations have their own Invalidate and OnInvalidate
functions, with an unexported onInvalidate field. However OnInvalidate and
Invalidate are usually not called in the same goroutine. This results in a race
on this field, e.g.:

    Read at 0x00c000094748 by goroutine 7:
      git.sr.ht/~sircmpwn/aerc2/widgets.NewDirectoryList.func1()
          /home/simon/src/aerc2/widgets/dirlist.go:85 +0x56
      git.sr.ht/~sircmpwn/aerc2/widgets.(*Spinner).Start.func1()
          /home/simon/src/aerc2/widgets/spinner.go:93 +0x1bb

    Previous write at 0x00c000094748 by main goroutine:
      [failed to restore the stack]

    Goroutine 7 (running) created at:
      git.sr.ht/~sircmpwn/aerc2/widgets.(*Spinner).Start()
          /home/simon/src/aerc2/widgets/spinner.go:46 +0x8f
      git.sr.ht/~sircmpwn/aerc2/widgets.NewDirectoryList()
          /home/simon/src/aerc2/widgets/dirlist.go:37 +0x286
      git.sr.ht/~sircmpwn/aerc2/widgets.NewAccountView()
          /home/simon/src/aerc2/widgets/account.go:50 +0x5ca
      git.sr.ht/~sircmpwn/aerc2/widgets.NewAerc()
          /home/simon/src/aerc2/widgets/aerc.go:60 +0x800
      main.main()
          /home/simon/src/aerc2/aerc.go:65 +0x33e

To fix this, introduce a new type, Invalidatable, which protects the field.
Unfortunately the Drawable must be passed to the callback function in
Invalidate, so we still need to re-implement this in each Invalidatable user.

Diffstat:
Mlib/ui/borders.go | 9++-------
Mlib/ui/grid.go | 14+++-----------
Alib/ui/invalidatable.go | 24++++++++++++++++++++++++
Mlib/ui/text.go | 22++++++++--------------
Mwidgets/account.go | 19+++++++++----------
Mwidgets/dirlist.go | 10++--------
Mwidgets/exline.go | 9++-------
Mwidgets/msglist.go | 24+++++++++---------------
Mwidgets/msgviewer.go | 21++++-----------------
Mwidgets/spinner.go | 10++--------
Mwidgets/status.go | 11++---------
Mwidgets/terminal.go | 36+++++++++++++++---------------------
12 files changed, 82 insertions(+), 127 deletions(-)

diff --git a/lib/ui/borders.go b/lib/ui/borders.go @@ -12,6 +12,7 @@ const ( ) type Bordered struct { + Invalidatable borders uint content Drawable onInvalidate func(d Drawable) @@ -35,13 +36,7 @@ func (bordered *Bordered) Children() []Drawable { } func (bordered *Bordered) Invalidate() { - if bordered.onInvalidate != nil { - bordered.onInvalidate(bordered) - } -} - -func (bordered *Bordered) OnInvalidate(onInvalidate func(d Drawable)) { - bordered.onInvalidate = onInvalidate + bordered.DoInvalidate(bordered) } func (bordered *Bordered) Draw(ctx *Context) { diff --git a/lib/ui/grid.go b/lib/ui/grid.go @@ -6,12 +6,12 @@ import ( ) type Grid struct { + Invalidatable rows []GridSpec rowLayout []gridLayout columns []GridSpec columnLayout []gridLayout cells []*GridCell - onInvalidate func(d Drawable) invalid bool } @@ -141,9 +141,7 @@ func (grid *Grid) reflow(ctx *Context) { func (grid *Grid) invalidateLayout() { grid.invalid = true - if grid.onInvalidate != nil { - grid.onInvalidate(grid) - } + grid.DoInvalidate(grid) } func (grid *Grid) Invalidate() { @@ -153,10 +151,6 @@ func (grid *Grid) Invalidate() { } } -func (grid *Grid) OnInvalidate(onInvalidate func(d Drawable)) { - grid.onInvalidate = onInvalidate -} - func (grid *Grid) AddChild(content Drawable) *GridCell { cell := &GridCell{ RowSpan: 1, @@ -193,7 +187,5 @@ func (grid *Grid) cellInvalidated(drawable Drawable) { panic(fmt.Errorf("Attempted to invalidate unknown cell")) } cell.invalid = true - if grid.onInvalidate != nil { - grid.onInvalidate(grid) - } + grid.DoInvalidate(grid) } diff --git a/lib/ui/invalidatable.go b/lib/ui/invalidatable.go @@ -0,0 +1,24 @@ +package ui + +import ( + "sync/atomic" +) + +type Invalidatable struct { + onInvalidate atomic.Value +} + +func (i *Invalidatable) OnInvalidate(f func(d Drawable)) { + i.onInvalidate.Store(f) +} + +func (i *Invalidatable) DoInvalidate(d Drawable) { + v := i.onInvalidate.Load() + if v == nil { + return + } + f := v.(func(d Drawable)) + if f != nil { + f(d) + } +} diff --git a/lib/ui/text.go b/lib/ui/text.go @@ -12,13 +12,13 @@ const ( ) type Text struct { - text string - strategy uint - fg tcell.Color - bg tcell.Color - bold bool - reverse bool - onInvalidate func(d Drawable) + Invalidatable + text string + strategy uint + fg tcell.Color + bg tcell.Color + bold bool + reverse bool } func NewText(text string) *Text { @@ -80,12 +80,6 @@ func (t *Text) Draw(ctx *Context) { ctx.Printf(x, 0, style, t.text) } -func (t *Text) OnInvalidate(onInvalidate func(d Drawable)) { - t.onInvalidate = onInvalidate -} - func (t *Text) Invalidate() { - if t.onInvalidate != nil { - t.onInvalidate(t) - } + t.DoInvalidate(t) } diff --git a/widgets/account.go b/widgets/account.go @@ -14,16 +14,15 @@ import ( ) type AccountView struct { - acct *config.AccountConfig - conf *config.AercConfig - dirlist *DirectoryList - grid *ui.Grid - host TabHost - logger *log.Logger - onInvalidate func(d ui.Drawable) - msglist *MessageList - msgStores map[string]*lib.MessageStore - worker *types.Worker + acct *config.AccountConfig + conf *config.AercConfig + dirlist *DirectoryList + grid *ui.Grid + host TabHost + logger *log.Logger + msglist *MessageList + msgStores map[string]*lib.MessageStore + worker *types.Worker } func NewAccountView(conf *config.AercConfig, acct *config.AccountConfig, diff --git a/widgets/dirlist.go b/widgets/dirlist.go @@ -12,10 +12,10 @@ import ( ) type DirectoryList struct { + ui.Invalidatable conf *config.AccountConfig dirs []string logger *log.Logger - onInvalidate func(d ui.Drawable) selecting string selected string spinner *Spinner @@ -77,14 +77,8 @@ func (dirlist *DirectoryList) Selected() string { return dirlist.selected } -func (dirlist *DirectoryList) OnInvalidate(onInvalidate func(d ui.Drawable)) { - dirlist.onInvalidate = onInvalidate -} - func (dirlist *DirectoryList) Invalidate() { - if dirlist.onInvalidate != nil { - dirlist.onInvalidate(dirlist) - } + dirlist.DoInvalidate(dirlist) } func (dirlist *DirectoryList) Draw(ctx *ui.Context) { diff --git a/widgets/exline.go b/widgets/exline.go @@ -12,6 +12,7 @@ import ( // TODO: scrolling type ExLine struct { + ui.Invalidatable command []rune commit func(cmd string) ctx *ui.Context @@ -33,14 +34,8 @@ func NewExLine(commit func(cmd string), cancel func()) *ExLine { } } -func (ex *ExLine) OnInvalidate(onInvalidate func(d ui.Drawable)) { - ex.onInvalidate = onInvalidate -} - func (ex *ExLine) Invalidate() { - if ex.onInvalidate != nil { - ex.onInvalidate(ex) - } + ex.DoInvalidate(ex) } func (ex *ExLine) Draw(ctx *ui.Context) { diff --git a/widgets/msglist.go b/widgets/msglist.go @@ -12,14 +12,14 @@ import ( ) type MessageList struct { - conf *config.AercConfig - logger *log.Logger - height int - onInvalidate func(d ui.Drawable) - scroll int - selected int - spinner *Spinner - store *lib.MessageStore + ui.Invalidatable + conf *config.AercConfig + logger *log.Logger + height int + scroll int + selected int + spinner *Spinner + store *lib.MessageStore } // TODO: fish in config @@ -37,14 +37,8 @@ func NewMessageList(logger *log.Logger) *MessageList { return ml } -func (ml *MessageList) OnInvalidate(onInvalidate func(d ui.Drawable)) { - ml.onInvalidate = onInvalidate -} - func (ml *MessageList) Invalidate() { - if ml.onInvalidate != nil { - ml.onInvalidate(ml) - } + ml.DoInvalidate(ml) } func (ml *MessageList) Draw(ctx *ui.Context) { diff --git a/widgets/msgviewer.go b/widgets/msgviewer.go @@ -252,8 +252,7 @@ func (mv *MessageViewer) Focus(focus bool) { } type HeaderView struct { - onInvalidate func(d ui.Drawable) - + ui.Invalidatable Name string Value string } @@ -281,17 +280,11 @@ func (hv *HeaderView) Draw(ctx *ui.Context) { } func (hv *HeaderView) Invalidate() { - if hv.onInvalidate != nil { - hv.onInvalidate(hv) - } -} - -func (hv *HeaderView) OnInvalidate(fn func(d ui.Drawable)) { - hv.onInvalidate = fn + hv.DoInvalidate(hv) } type MultipartView struct { - onInvalidate func(d ui.Drawable) + ui.Invalidatable } func (mpv *MultipartView) Draw(ctx *ui.Context) { @@ -303,11 +296,5 @@ func (mpv *MultipartView) Draw(ctx *ui.Context) { } func (mpv *MultipartView) Invalidate() { - if mpv.onInvalidate != nil { - mpv.onInvalidate(mpv) - } -} - -func (mpv *MultipartView) OnInvalidate(fn func(d ui.Drawable)) { - mpv.onInvalidate = fn + mpv.DoInvalidate(mpv) } diff --git a/widgets/spinner.go b/widgets/spinner.go @@ -23,8 +23,8 @@ var ( ) type Spinner struct { + ui.Invalidatable frame int64 // access via atomic - onInvalidate func(d ui.Drawable) stop chan struct{} } @@ -84,12 +84,6 @@ func (s *Spinner) Draw(ctx *ui.Context) { ctx.Printf(col, 0, tcell.StyleDefault, "%s", frames[cur]) } -func (s *Spinner) OnInvalidate(onInvalidate func(d ui.Drawable)) { - s.onInvalidate = onInvalidate -} - func (s *Spinner) Invalidate() { - if s.onInvalidate != nil { - s.onInvalidate(s) - } + s.DoInvalidate(s) } diff --git a/widgets/status.go b/widgets/status.go @@ -9,10 +9,9 @@ import ( ) type StatusLine struct { + ui.Invalidatable stack []*StatusMessage fallback StatusMessage - - onInvalidate func(d ui.Drawable) } type StatusMessage struct { @@ -31,14 +30,8 @@ func NewStatusLine() *StatusLine { } } -func (status *StatusLine) OnInvalidate(onInvalidate func(d ui.Drawable)) { - status.onInvalidate = onInvalidate -} - func (status *StatusLine) Invalidate() { - if status.onInvalidate != nil { - status.onInvalidate(status) - } + status.DoInvalidate(status) } func (status *StatusLine) Draw(ctx *ui.Context) { diff --git a/widgets/terminal.go b/widgets/terminal.go @@ -88,20 +88,20 @@ func init() { } type Terminal struct { - closed bool - cmd *exec.Cmd - colors map[tcell.Color]tcell.Color - ctx *ui.Context - cursorPos vterm.Pos - cursorShown bool - damage []vterm.Rect - destroyed bool - err error - focus bool - onInvalidate func(d ui.Drawable) - pty *os.File - start chan interface{} - vterm *vterm.VTerm + ui.Invalidatable + closed bool + cmd *exec.Cmd + colors map[tcell.Color]tcell.Color + ctx *ui.Context + cursorPos vterm.Pos + cursorShown bool + damage []vterm.Rect + destroyed bool + err error + focus bool + pty *os.File + start chan interface{} + vterm *vterm.VTerm OnClose func(err error) OnStart func() @@ -225,10 +225,6 @@ func (term *Terminal) Destroy() { term.destroyed = true } -func (term *Terminal) OnInvalidate(cb func(d ui.Drawable)) { - term.onInvalidate = cb -} - func (term *Terminal) Invalidate() { if term.vterm != nil { width, height := term.vterm.Size() @@ -239,9 +235,7 @@ func (term *Terminal) Invalidate() { } func (term *Terminal) invalidate() { - if term.onInvalidate != nil { - term.onInvalidate(term) - } + term.DoInvalidate(term) } func (term *Terminal) Draw(ctx *ui.Context) {