From 5685a176747711d0330f1251ad297b13e0621a37 Mon Sep 17 00:00:00 2001 From: Simon Ser Date: Sat, 27 Apr 2019 16:47:59 +0000 Subject: 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. --- lib/ui/borders.go | 9 ++------- lib/ui/grid.go | 14 +++----------- lib/ui/invalidatable.go | 24 ++++++++++++++++++++++++ lib/ui/text.go | 22 ++++++++-------------- widgets/account.go | 19 +++++++++---------- widgets/dirlist.go | 10 ++-------- widgets/exline.go | 9 ++------- widgets/msglist.go | 24 +++++++++--------------- widgets/msgviewer.go | 21 ++++----------------- widgets/spinner.go | 10 ++-------- widgets/status.go | 11 ++--------- widgets/terminal.go | 36 +++++++++++++++--------------------- 12 files changed, 82 insertions(+), 127 deletions(-) create mode 100644 lib/ui/invalidatable.go diff --git a/lib/ui/borders.go b/lib/ui/borders.go index 9b7860e..cffd3ca 100644 --- 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 index 87b94bd..3ac43a4 100644 --- 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 new file mode 100644 index 0000000..9275712 --- /dev/null +++ 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 index 761673c..8aea8eb 100644 --- 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 index f8abdc9..a8cd9ad 100644 --- 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 index eb79bc4..374d142 100644 --- 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 index 5c9f065..8b18736 100644 --- 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 index ea9b245..c4b5d82 100644 --- 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 index 711879b..ba99911 100644 --- 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 index 0ab3e13..bb7dbe8 100644 --- 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 index 3536760..344454a 100644 --- 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 index 84f80e3..63c105f 100644 --- 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) { -- cgit v1.2.3