Merge pull request #5384 from andydotxyz/fix/racedetection

That's the races out of our test suite!

.github/workflows/mobile_tests.yml

@@ -10,6 +10,9 @@ jobs:
       fail-fast: false
       matrix:
         go-version: ['1.19.x', '1.23.x']
+        include:
+          - os: ubuntu-latest
+            runner: xvfb-run
 
     steps:
     - uses: actions/checkout@v4
@@ -23,4 +26,4 @@ jobs:
       run: sudo apt-get update && sudo apt-get install gcc libegl1-mesa-dev libgles2-mesa-dev libx11-dev xorg-dev
 
     - name: Tests
-      run: go test -test.benchtime 10ms -tags "ci mobile" ./...
+      run: go test -race -test.benchtime 10ms -tags "ci mobile" ./...

.github/workflows/platform_tests.yml

@@ -40,10 +40,10 @@ jobs:
       if: ${{ runner.os == 'Linux' }}
 
     - name: Tests
-      run: ${{ matrix.runner }} go test "-test.benchtime" 10ms -tags "${{ matrix.tags }}" ./...
+      run: ${{ matrix.runner }} go test -race "-test.benchtime" 10ms -tags "${{ matrix.tags }}" ./...
 
     - name: Wayland Tests
-      run: go test -tags no_glfw,ci,wayland ./...
+      run: go test -race -tags no_glfw,ci,wayland ./...
       if: ${{ runner.os == 'Linux' }}
 
   windows_tests:
@@ -61,4 +61,4 @@ jobs:
           go-version: ${{ matrix.go-version }}
 
       - name: Tests
-        run: ${{ matrix.runner }} go test "-test.benchtime" 10ms -tags no_glfw ./...
+        run: ${{ matrix.runner }} go test -race "-test.benchtime" 10ms -tags no_glfw ./...

app/app.go

@@ -68,7 +68,7 @@ func (a *fyneApp) NewWindow(title string) fyne.Window {
 }
 
 func (a *fyneApp) Run() {
-	go a.lifecycle.RunEventQueue()
+	go a.lifecycle.RunEventQueue(a.driver.CallFromGoroutine)
 	a.driver.Run()
 }
 

app/cloud_test.go

@@ -7,13 +7,14 @@ import (
 	"fyne.io/fyne/v2"
 	"fyne.io/fyne/v2/internal"
 	"fyne.io/fyne/v2/storage"
+	"fyne.io/fyne/v2/test"
 	"fyne.io/fyne/v2/theme"
 
 	"github.com/stretchr/testify/assert"
 )
 
 func TestFyneApp_SetCloudProvider(t *testing.T) {
-	a := NewWithID("io.fyne.test")
+	a := test.NewTempApp(t)
 	p := &mockCloud{}
 	a.SetCloudProvider(p)
 
@@ -22,7 +23,7 @@ func TestFyneApp_SetCloudProvider(t *testing.T) {
 }
 
 func TestFyneApp_SetCloudProvider_Cleanup(t *testing.T) {
-	a := NewWithID("io.fyne.test")
+	a := test.NewTempApp(t)
 	p1 := &mockCloud{}
 	p2 := &mockCloud{}
 	a.SetCloudProvider(p1)
@@ -37,22 +38,28 @@ func TestFyneApp_SetCloudProvider_Cleanup(t *testing.T) {
 }
 
 func TestFyneApp_transitionCloud(t *testing.T) {
-	a := NewWithID("io.fyne.test")
+	a := test.NewTempApp(t)
 	p := &mockCloud{}
 	preferenceChanged := false
 	settingsChan := make(chan fyne.Settings)
 	a.Preferences().AddChangeListener(func() {
 		preferenceChanged = true
 	})
 	a.Settings().AddChangeListener(settingsChan)
-	a.SetCloudProvider(p)
 
-	<-settingsChan // settings were updated
-	assert.True(t, preferenceChanged)
+	done := make(chan struct{})
+	go func() {
+		<-settingsChan // settings were updated
+		assert.True(t, preferenceChanged)
+		close(done)
+	}()
+
+	a.SetCloudProvider(p)
+	<-done
 }
 
 func TestFyneApp_transitionCloud_Preferences(t *testing.T) {
-	a := NewWithID("io.fyne.test")
+	a := test.NewTempApp(t)
 	a.Preferences().SetString("key", "blank")
 
 	assert.Equal(t, "blank", a.Preferences().String("key"))
@@ -64,7 +71,7 @@ func TestFyneApp_transitionCloud_Preferences(t *testing.T) {
 }
 
 func TestFyneApp_transitionCloud_Storage(t *testing.T) {
-	a := NewWithID("io.fyne.test")
+	a := test.NewTempApp(t)
 	a.Storage().Create("nothere")
 
 	l := a.Storage().List()

app/preferences.go

@@ -50,15 +50,19 @@ func (p *preferences) forceImmediateSave() {
 func (p *preferences) resetSavedRecently() {
 	go func() {
 		time.Sleep(time.Millisecond * 100) // writes are not always atomic. 10ms worked, 100 is safer.
-		p.prefLock.Lock()
-		p.savedRecently = false
-		changedDuringSaving := p.changedDuringSaving
-		p.changedDuringSaving = false
-		p.prefLock.Unlock()
 
-		if changedDuringSaving {
-			p.save()
-		}
+		// For test reasons we need to use current app not what we were initialised with as they can differ
+		fyne.CurrentApp().Driver().CallFromGoroutine(func() {
+			p.prefLock.Lock()
+			p.savedRecently = false
+			changedDuringSaving := p.changedDuringSaving
+			p.changedDuringSaving = false
+			p.prefLock.Unlock()
+
+			if changedDuringSaving {
+				p.save()
+			}
+		})
 	}()
 }
 

app/preferences_other.go

@@ -5,6 +5,7 @@ package app
 import (
 	"path/filepath"
 
+	"fyne.io/fyne/v2"
 	"fyne.io/fyne/v2/internal/app"
 )
 
@@ -27,6 +28,6 @@ func (p *preferences) watch() {
 			return
 		}
 
-		p.load()
+		fyne.CurrentApp().Driver().CallFromGoroutine(p.load)
 	})
 }

data/binding/pref_helper.go

@@ -1,6 +1,8 @@
 package binding
 
 import (
+	"sync"
+
 	"fyne.io/fyne/v2"
 	"fyne.io/fyne/v2/internal/async"
 )
@@ -26,6 +28,7 @@ type preferencesMap struct {
 	prefs async.Map[fyne.Preferences, *preferenceBindings]
 
 	appPrefs fyne.Preferences // the main application prefs, to check if it changed...
+	appLock  sync.Mutex
 }
 
 func newPreferencesMap() *preferencesMap {
@@ -44,10 +47,14 @@ func (m *preferencesMap) ensurePreferencesAttached(p fyne.Preferences) *preferen
 
 func (m *preferencesMap) getBindings(p fyne.Preferences) *preferenceBindings {
 	if p == fyne.CurrentApp().Preferences() {
+		m.appLock.Lock()
+		prefs := m.appPrefs
 		if m.appPrefs == nil {
 			m.appPrefs = p
-		} else if m.appPrefs != p {
-			m.migratePreferences(m.appPrefs, p)
+		}
+		m.appLock.Unlock()
+		if prefs != p {
+			m.migratePreferences(prefs, p)
 		}
 	}
 	binds, _ := m.prefs.Load(p)
@@ -72,7 +79,9 @@ func (m *preferencesMap) migratePreferences(src, dst fyne.Preferences) {
 
 	m.prefs.Store(dst, old)
 	m.prefs.Delete(src)
+	m.appLock.Lock()
 	m.appPrefs = dst
+	m.appLock.Unlock()
 
 	binds := m.getBindings(dst)
 	if binds == nil {

dialog/file.go

@@ -946,6 +946,10 @@ func getFavoriteOrder() []string {
 }
 
 func hasAppFiles(a fyne.App) bool {
+	if a.UniqueID() == "testApp" {
+		return false
+	}
+
 	return len(a.Storage().List()) > 0
 }
 

dialog/file_test.go

@@ -659,6 +659,7 @@ func TestViewPreferences(t *testing.T) {
 }
 
 func TestFileFavorites(t *testing.T) {
+	_ = test.NewApp()
 	win := test.NewTempWindow(t, widget.NewLabel("Content"))
 
 	dlg := NewFileOpen(func(reader fyne.URIReadCloser, err error) {

internal/animation/animation_test.go

@@ -99,10 +99,10 @@ func TestGLDriver_StopAnimationImmediatelyAndInsideTick(t *testing.T) {
 		Tick:     func(f float32) {},
 	}
 	run.Start(c)
-	go tick(run) // simulate a graphics draw loop
+	tick(run) // simulate a graphics draw loop
 
 	run.Stop(c)
-	go tick(run) // simulate a graphics draw loop
+	tick(run) // simulate a graphics draw loop
 
 	wg.Wait()
 	// animations stopped inside tick are really stopped in the next runner cycle

internal/app/lifecycle.go

@@ -123,9 +123,9 @@ func (l *Lifecycle) QueueEvent(fn func()) {
 
 // RunEventQueue runs the event queue. This should called inside a go routine.
 // This function blocks.
-func (l *Lifecycle) RunEventQueue() {
+func (l *Lifecycle) RunEventQueue(run func(func())) {
 	for fn := range l.eventQueue.Out() {
-		fn()
+		run(fn)
 	}
 }
 

internal/app/lifecycle_test.go

@@ -17,7 +17,9 @@ func TestLifecycle(t *testing.T) {
 
 	var entered, exited, start, stop, hookedStop, called bool
 	life.InitEventQueue()
-	go life.RunEventQueue()
+	go life.RunEventQueue(func(fn func()) {
+		fn()
+	})
 	life.QueueEvent(func() { called = true })
 	life.SetOnEnteredForeground(func() { entered = true })
 	life.OnEnteredForeground()()

internal/async/pool_test.go

@@ -12,10 +12,6 @@ func TestPool(t *testing.T) {
 	item := pool.Get()
 	assert.Equal(t, 0, item)
 
-	item = 5
-	pool.Put(item)
-	assert.Equal(t, item, pool.Get())
-
 	pool.New = func() int {
 		return -1
 	}

internal/driver/glfw/canvas_other_test.go

@@ -14,13 +14,15 @@ import (
 func TestGlCanvas_FocusHandlingWhenActivatingOrDeactivatingTheMenu(t *testing.T) {
 	w := createWindow("Test")
 	w.SetPadded(false)
-	w.SetMainMenu(
-		fyne.NewMainMenu(
-			fyne.NewMenu("test", fyne.NewMenuItem("item", func() {})),
-			fyne.NewMenu("other", fyne.NewMenuItem("item", func() {})),
-		),
-	)
-	c := w.Canvas().(*glCanvas)
+	runOnMain(func() {
+		w.SetMainMenu(
+			fyne.NewMainMenu(
+				fyne.NewMenu("test", fyne.NewMenuItem("item", func() {})),
+				fyne.NewMenu("other", fyne.NewMenuItem("item", func() {})),
+			),
+		)
+	})
+	c := w.Canvas()
 
 	ce1 := &focusable{id: "ce1"}
 	ce2 := &focusable{id: "ce2"}
@@ -35,7 +37,9 @@ func TestGlCanvas_FocusHandlingWhenActivatingOrDeactivatingTheMenu(t *testing.T)
 	assert.Equal(t, ce2, c.Focused())
 	assert.True(t, ce2.focused)
 
-	m.Items[0].(*menuBarItem).Tapped(&fyne.PointEvent{})
+	runOnMain(func() {
+		m.Items[0].(*menuBarItem).Tapped(&fyne.PointEvent{})
+	})
 	assert.True(t, m.IsActive())
 	ctxt := "activating the menu changes focus handler and focuses the menu bar item but does not remove focus from content"
 	assert.True(t, ce2.focused, ctxt)
@@ -46,7 +50,9 @@ func TestGlCanvas_FocusHandlingWhenActivatingOrDeactivatingTheMenu(t *testing.T)
 	assert.True(t, ce2.focused, ctxt)
 	assert.Equal(t, m.Items[1], c.Focused(), ctxt)
 
-	m.Items[0].(*menuBarItem).Tapped(&fyne.PointEvent{})
+	runOnMain(func() {
+		m.Items[0].(*menuBarItem).Tapped(&fyne.PointEvent{})
+	})
 	assert.False(t, m.IsActive())
 	ctxt = "deactivating the menu restores focus handler from content"
 	assert.True(t, ce2.focused, ctxt)