lib/ui: fix Grid race condition
This was is more complicated than others. The cells list is accessed by multiple goroutines: - Some change the Grid's contents via AddChild/RemoveChild - Some call Draw - Some invalidate the grid via Invalidate Invalidate calls are tricky to handle because they will also invalidate all child cells. This will inturn trigger the cellInvalidated callback, which needs to read the list of cells. For this reason, we use a sync.RWLock which allows multiple concurrent reads. Below is the race fixed by this commit. Read at 0x00c0000bc3d0 by goroutine 7: git.sr.ht/~sircmpwn/aerc2/lib/ui.(*Grid).cellInvalidated() /home/simon/src/aerc2/lib/ui/grid.go:181 +0x45 git.sr.ht/~sircmpwn/aerc2/lib/ui.(*Grid).cellInvalidated-fm() /home/simon/src/aerc2/lib/ui/grid.go:179 +0x55 git.sr.ht/~sircmpwn/aerc2/lib/ui.(*Invalidatable).DoInvalidate() /home/simon/src/aerc2/lib/ui/invalidatable.go:22 +0x85 git.sr.ht/~sircmpwn/aerc2/lib/ui.(*Bordered).contentInvalidated-fm() /home/simon/src/aerc2/lib/ui/borders.go:39 +0x56 git.sr.ht/~sircmpwn/aerc2/lib/ui.(*Invalidatable).DoInvalidate() /home/simon/src/aerc2/lib/ui/invalidatable.go:22 +0x85 git.sr.ht/~sircmpwn/aerc2/widgets.NewDirectoryList.func1() /home/simon/src/aerc2/widgets/dirlist.go:81 +0x55 git.sr.ht/~sircmpwn/aerc2/lib/ui.(*Invalidatable).DoInvalidate() /home/simon/src/aerc2/lib/ui/invalidatable.go:22 +0x85 git.sr.ht/~sircmpwn/aerc2/widgets.(*Spinner).Start.func1() /home/simon/src/aerc2/widgets/spinner.go:88 +0x82 Previous write at 0x00c0000bc3d0 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 +0x98 git.sr.ht/~sircmpwn/aerc2/widgets.NewDirectoryList() /home/simon/src/aerc2/widgets/dirlist.go:37 +0x28b git.sr.ht/~sircmpwn/aerc2/widgets.NewAccountView() /home/simon/src/aerc2/widgets/account.go:49 +0x5ca git.sr.ht/~sircmpwn/aerc2/widgets.NewAerc() /home/simon/src/aerc2/widgets/aerc.go:60 +0x807 main.main() /home/simon/src/aerc2/aerc.go:65 +0x33e
This commit is contained in:
parent
a275f65848
commit
5feb7dede9
|
@ -3,6 +3,7 @@ package ui
|
|||
import (
|
||||
"fmt"
|
||||
"math"
|
||||
"sync"
|
||||
"sync/atomic"
|
||||
)
|
||||
|
||||
|
@ -12,8 +13,11 @@ type Grid struct {
|
|||
rowLayout []gridLayout
|
||||
columns []GridSpec
|
||||
columnLayout []gridLayout
|
||||
cells []*GridCell
|
||||
invalid bool
|
||||
|
||||
// Protected by mutex
|
||||
cells []*GridCell
|
||||
mutex sync.RWMutex
|
||||
}
|
||||
|
||||
const (
|
||||
|
@ -73,6 +77,9 @@ func (grid *Grid) Columns(spec []GridSpec) *Grid {
|
|||
}
|
||||
|
||||
func (grid *Grid) Children() []Drawable {
|
||||
grid.mutex.RLock()
|
||||
defer grid.mutex.RUnlock()
|
||||
|
||||
children := make([]Drawable, len(grid.cells))
|
||||
for i, cell := range grid.cells {
|
||||
children[i] = cell.Content
|
||||
|
@ -85,6 +92,10 @@ func (grid *Grid) Draw(ctx *Context) {
|
|||
if invalid {
|
||||
grid.reflow(ctx)
|
||||
}
|
||||
|
||||
grid.mutex.RLock()
|
||||
defer grid.mutex.RUnlock()
|
||||
|
||||
for _, cell := range grid.cells {
|
||||
cellInvalid := cell.invalid.Load().(bool)
|
||||
if !cellInvalid && !invalid {
|
||||
|
@ -148,9 +159,11 @@ func (grid *Grid) invalidateLayout() {
|
|||
|
||||
func (grid *Grid) Invalidate() {
|
||||
grid.invalidateLayout()
|
||||
grid.mutex.RLock()
|
||||
for _, cell := range grid.cells {
|
||||
cell.Content.Invalidate()
|
||||
}
|
||||
grid.mutex.RUnlock()
|
||||
}
|
||||
|
||||
func (grid *Grid) AddChild(content Drawable) *GridCell {
|
||||
|
@ -159,7 +172,9 @@ func (grid *Grid) AddChild(content Drawable) *GridCell {
|
|||
ColSpan: 1,
|
||||
Content: content,
|
||||
}
|
||||
grid.mutex.Lock()
|
||||
grid.cells = append(grid.cells, cell)
|
||||
grid.mutex.Unlock()
|
||||
cell.Content.OnInvalidate(grid.cellInvalidated)
|
||||
cell.invalid.Store(true)
|
||||
grid.invalidateLayout()
|
||||
|
@ -167,23 +182,27 @@ func (grid *Grid) AddChild(content Drawable) *GridCell {
|
|||
}
|
||||
|
||||
func (grid *Grid) RemoveChild(cell *GridCell) {
|
||||
grid.mutex.Lock()
|
||||
for i, _cell := range grid.cells {
|
||||
if _cell == cell {
|
||||
grid.cells = append(grid.cells[:i], grid.cells[i+1:]...)
|
||||
break
|
||||
}
|
||||
}
|
||||
grid.mutex.Unlock()
|
||||
grid.invalidateLayout()
|
||||
}
|
||||
|
||||
func (grid *Grid) cellInvalidated(drawable Drawable) {
|
||||
var cell *GridCell
|
||||
grid.mutex.RLock()
|
||||
for _, cell = range grid.cells {
|
||||
if cell.Content == drawable {
|
||||
break
|
||||
}
|
||||
cell = nil
|
||||
}
|
||||
grid.mutex.RUnlock()
|
||||
if cell == nil {
|
||||
panic(fmt.Errorf("Attempted to invalidate unknown cell"))
|
||||
}
|
||||
|
|
Loading…
Reference in New Issue