Concurrent map writes in main loop

[steveoc64]

Whilst running main.go in github.com/fyne-io/fyne/examples

Clicking on a few things, bring up the canvas .. boom.

2018/09/16 19:37:53 DPI 157
fatal error: concurrent map writes

goroutine 1 [running, locked to thread]:
runtime.throw(0x844a60, 0x15)
        /usr/local/go/src/runtime/panic.go:608 +0x72 fp=0xc000107aa8 sp=0xc000107a78 pc=0x42e7a2
runtime.mapassign(0x7c4920, 0xc0001c85d0, 0xc000107b80, 0x8)
        /usr/local/go/src/runtime/map.go:563 +0x560 fp=0xc000107b30 sp=0xc000107aa8 pc=0x411f60
github.com/fyne-io/fyne/desktop.queueRender(...)
        /home/steve/go/src/github.com/fyne-io/fyne/desktop/loop.go:77
github.com/fyne-io/fyne/desktop.(*eflCanvas).resizeContent(0xc0001ca100)
        /home/steve/go/src/github.com/fyne-io/fyne/desktop/canvas.go:429 +0x1a4 fp=0xc000107ba0 sp=0xc000107b30 pc=0x7583e4
.......

Not easily reproducable, because its just a matter of bad luck that something adds to the map whilst loop.go is deleting something from the map.

Wrapping the eflcanvas.dirty map in a mutex makes the problem go away, so thats one solution, as ugly as it is. Does not affect performance to any noticeable degree.

There are 4 maps all up in the eflcanvas struct including the dirty map .. not sure if some of these will have similar issues. Will have a closer look.

From 61680d2744aa3a5fa22d68bc9c19d6430454bb8f Mon Sep 17 00:00:00 2001
From: SteveOC <steven.oconnor@neds.com>
Date: Sun, 16 Sep 2018 19:47:31 +1000
Subject: [PATCH] add mutex around canvas/dirty or concurrent map writes

---
 desktop/canvas.go | 9 +++++----
 desktop/loop.go   | 4 ++++
 2 files changed, 9 insertions(+), 4 deletions(-)

diff --git a/desktop/canvas.go b/desktop/canvas.go
index a97e8c1..eaaecd6 100644
--- a/desktop/canvas.go
+++ b/desktop/canvas.go
@@ -59,10 +59,11 @@ type eflCanvas struct {
 
        onKeyDown func(*fyne.KeyEvent)
 
-       objects map[*C.Evas_Object]fyne.CanvasObject
-       native  map[fyne.CanvasObject]*C.Evas_Object
-       offsets map[fyne.CanvasObject]fyne.Position
-       dirty   map[fyne.CanvasObject]bool
+       objects    map[*C.Evas_Object]fyne.CanvasObject
+       native     map[fyne.CanvasObject]*C.Evas_Object
+       offsets    map[fyne.CanvasObject]fyne.Position
+       dirty      map[fyne.CanvasObject]bool
+       dirtyMutex sync.Mutex
 }
 
 func ignoreObject(o fyne.CanvasObject) bool {
diff --git a/desktop/loop.go b/desktop/loop.go
index b3412ff..fcd7167 100644
--- a/desktop/loop.go
+++ b/desktop/loop.go
@@ -64,15 +64,19 @@ func drawDirty() {
                        continue
                }
                canvas.fitContent()
+               canvas.dirtyMutex.Lock()
                for obj := range canvas.dirty {
                        delete(canvas.dirty, obj)
 
                        canvas.doRefresh(obj)
                }
+               canvas.dirtyMutex.Unlock()
        }
 }
 
 // do runs f on the main thread.
 func queueRender(c *eflCanvas, co fyne.CanvasObject) {
+       c.dirtyMutex.Lock()
        c.dirty[co] = true
+       c.dirtyMutex.Unlock()
 }
-- 
2.18.0

[andydotxyz]

Thanks for your report, I hope we can make this thread handling really robust so that Fyne users never have to worry!

Following a discussion on Slack it seems like we might be able to find a better solution where a channel is used to avoid the other threads using the dirty map at all.
Let's investigate further.

[andydotxyz]

This was addressed by #19