From 5685a176747711d0330f1251ad297b13e0621a37 Mon Sep 17 00:00:00 2001
From: Simon Ser <contact@emersion.fr>
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 ++++++++--------------
 4 files changed, 37 insertions(+), 32 deletions(-)
 create mode 100644 lib/ui/invalidatable.go

(limited to 'lib/ui')

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)
 }
-- 
cgit 1.4.1-2-gfad0