about summary refs log tree commit diff stats
diff options
context:
space:
mode:
authorSimon Ser <contact@emersion.fr>2019-04-27 16:47:59 +0000
committerDrew DeVault <sir@cmpwn.com>2019-04-27 14:30:28 -0400
commit5685a176747711d0330f1251ad297b13e0621a37 (patch)
treed338db2be97f5ac412a427776167d21b04abdf99
parent9ef2a57b51601cc0e18824bbfe8026891da1a38d (diff)
downloadaerc-5685a176747711d0330f1251ad297b13e0621a37.tar.gz
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.
-rw-r--r--lib/ui/borders.go9
-rw-r--r--lib/ui/grid.go14
-rw-r--r--lib/ui/invalidatable.go24
-rw-r--r--lib/ui/text.go22
-rw-r--r--widgets/account.go19
-rw-r--r--widgets/dirlist.go10
-rw-r--r--widgets/exline.go9
-rw-r--r--widgets/msglist.go24
-rw-r--r--widgets/msgviewer.go21
-rw-r--r--widgets/spinner.go10
-rw-r--r--widgets/status.go11
-rw-r--r--widgets/terminal.go36
12 files changed, 82 insertions, 127 deletions
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) {