Merge pull request #2048 from fpabl0/fix/race-glfw

Fix several race conditions in internal/driver/glfw package

internal/driver/glfw/canvas.go

@@ -66,8 +66,11 @@ func (c *glCanvas) Content() fyne.CanvasObject {
 }
 
 func (c *glCanvas) DismissMenu() bool {
-	if c.menu != nil && c.menu.(*MenuBar).IsActive() {
-		c.menu.(*MenuBar).Toggle()
+	c.RLock()
+	menu := c.menu
+	c.RUnlock()
+	if menu != nil && menu.(*MenuBar).IsActive() {
+		menu.(*MenuBar).Toggle()
 		return true
 	}
 	return false
@@ -154,7 +157,9 @@ func (c *glCanvas) Padded() bool {
 }
 
 func (c *glCanvas) PixelCoordinateForPosition(pos fyne.Position) (int, int) {
+	c.RLock()
 	texScale := c.texScale
+	c.RUnlock()
 	multiple := c.Scale() * texScale
 	scaleInt := func(x float32) int {
 		return int(math.Round(float64(x * multiple)))
@@ -243,7 +248,11 @@ func (c *glCanvas) SetPadded(padded bool) {
 }
 
 func (c *glCanvas) reloadScale() {
-	if !c.context.(*window).visible {
+	w := c.context.(*window)
+	w.viewLock.RLock()
+	windowVisible := w.visible
+	w.viewLock.RUnlock()
+	if !windowVisible {
 		return
 	}
 
@@ -262,8 +271,11 @@ func (c *glCanvas) Size() fyne.Size {
 }
 
 func (c *glCanvas) ToggleMenu() {
-	if c.menu != nil {
-		c.menu.(*MenuBar).Toggle()
+	c.RLock()
+	menu := c.menu
+	c.RUnlock()
+	if menu != nil {
+		menu.(*MenuBar).Toggle()
 	}
 }
 
@@ -407,19 +419,23 @@ func (c *glCanvas) menuHeight() float32 {
 }
 
 func (c *glCanvas) objectTrees() []fyne.CanvasObject {
+	c.RLock()
+	content := c.content
+	menu := c.menu
+	c.RUnlock()
 	trees := make([]fyne.CanvasObject, 0, len(c.Overlays().List())+2)
-	trees = append(trees, c.content)
-	if c.menu != nil {
-		trees = append(trees, c.menu)
+	trees = append(trees, content)
+	if menu != nil {
+		trees = append(trees, menu)
 	}
 	trees = append(trees, c.Overlays().List()...)
 	return trees
 }
 
 func (c *glCanvas) overlayChanged() {
-	c.Lock()
-	defer c.Unlock()
+	c.dirtyMutex.Lock()
 	c.dirty = true
+	c.dirtyMutex.Unlock()
 }
 
 func (c *glCanvas) paint(size fyne.Size) {
@@ -468,9 +484,8 @@ func (c *glCanvas) setContent(content fyne.CanvasObject) {
 
 func (c *glCanvas) setDirty(dirty bool) {
 	c.dirtyMutex.Lock()
-	defer c.dirtyMutex.Unlock()
-
 	c.dirty = dirty
+	c.dirtyMutex.Unlock()
 }
 
 func (c *glCanvas) setMenuOverlay(b fyne.CanvasObject) {
@@ -493,11 +508,15 @@ func (c *glCanvas) setupThemeListener() {
 	go func() {
 		for {
 			<-listener
-			if c.menu != nil {
-				app.ApplyThemeTo(c.menu, c) // Ensure our menu gets the theme change message as it's out-of-tree
+			c.RLock()
+			menu := c.menu
+			padded := c.padded
+			c.RUnlock()
+			if menu != nil {
+				app.ApplyThemeTo(menu, c) // Ensure our menu gets the theme change message as it's out-of-tree
 			}
 
-			c.SetPadded(c.padded) // refresh the padding for potential theme differences
+			c.SetPadded(padded) // refresh the padding for potential theme differences
 		}
 	}()
 }

internal/driver/glfw/canvas_test.go

@@ -366,12 +366,16 @@ func TestGlCanvas_PixelCoordinateAtPosition(t *testing.T) {
 	c := w.Canvas().(*glCanvas)
 
 	pos := fyne.NewPos(4, 4)
+	c.Lock()
 	c.scale = 2.5
+	c.Unlock()
 	x, y := c.PixelCoordinateForPosition(pos)
 	assert.Equal(t, int(10*c.texScale), x)
 	assert.Equal(t, int(10*c.texScale), y)
 
+	c.Lock()
 	c.texScale = 2.0
+	c.Unlock()
 	x, y = c.PixelCoordinateForPosition(pos)
 	assert.Equal(t, 20, x)
 	assert.Equal(t, 20, y)
@@ -487,7 +491,9 @@ func TestGlCanvas_Scale(t *testing.T) {
 	w := createWindow("Test").(*window)
 	c := w.Canvas().(*glCanvas)
 
+	c.Lock()
 	c.scale = 2.5
+	c.Unlock()
 	assert.Equal(t, 5, int(2*c.Scale()))
 }
 

internal/driver/glfw/driver_test.go

@@ -41,7 +41,9 @@ func Test_gLDriver_AbsolutePositionForObject(t *testing.T) {
 	// We work around w.SetMainMenu because on MacOS the main menu is a native menu.
 	c := w.Canvas().(*glCanvas)
 	movl := buildMenuOverlay(mm, c)
+	c.Lock()
 	c.setMenuOverlay(movl)
+	c.Unlock()
 	w.SetContent(content)
 	w.Resize(fyne.NewSize(200, 199))