From 0921509e265833f085f86aa5d1ebb67e14c30258 Mon Sep 17 00:00:00 2001
From: Abdelhafidh Belalia <16493223+s77rt@users.noreply.github.com>
Date: Mon, 24 May 2021 20:03:55 +0100
Subject: [PATCH] Wait for goroutines to finish when firing a change to an app
 preferences  (#2246)

Mainly there was two problems:
- the first and probably the main bug is that sometimes the app closes too early without waiting for the goroutines that were called in [fireChange()](https://github.com/fyne-io/fyne/blob/3814a66f75ff5d24be6fa107f1dfb9c4faa859b1/internal/preferences.go#L69) and one of those goroutines is the one that calls [save()](https://github.com/fyne-io/fyne/blob/3814a66f75ff5d24be6fa107f1dfb9c4faa859b1/app/preferences.go#L35) an easy fix was to implement the sync.WaitGroup functionality which i expected to work just fine, yet it didn't due to
- the second issue which was a misuse of the lockers, in lines [70 and 71](https://github.com/fyne-io/fyne/blob/3814a66f75ff5d24be6fa107f1dfb9c4faa859b1/internal/preferences.go#L70-L71) are using Lock() and after that there are goroutines that are expected to read the same struct, well they won't be able to because Lock() only allows one goroutine (read/write) at a time and in fact the goroutines functions that were thought are being executed were actually not, they were just in stuck position waiting for the fireChange() to return (and to trigger it's defer function to Unlock()) so all those go l() were only waiting for fireChange() to return then get executed, well after the fix (1) the function will never return because it's waiting for the goroutines functions which will never work either because they are waiting for fireChange() ,,, deadlock ; the fix was pretty clear is to use RLock() instead of Lock() that way, where multiple goroutines can read(but not write) the struct and the fireChange() will only returns after all the goroutines are done and only after that the app will close

Fixes #2241
---
 internal/preferences.go | 21 +++++++++++++++------
 1 file changed, 15 insertions(+), 6 deletions(-)

diff --git a/internal/preferences.go b/internal/preferences.go
index 1032d4f68a..b12387858c 100644
--- a/internal/preferences.go
+++ b/internal/preferences.go
@@ -11,12 +11,14 @@ type InMemoryPreferences struct {
 	values          map[string]interface{}
 	lock            sync.RWMutex
 	changeListeners []func()
+	wg              *sync.WaitGroup
 }
 
 // Declare conformity with Preferences interface
 var _ fyne.Preferences = (*InMemoryPreferences)(nil)
 
 // AddChangeListener allows code to be notified when some preferences change. This will fire on any update.
+// The passed 'listener' should not try to write values.
 func (p *InMemoryPreferences) AddChangeListener(listener func()) {
 	p.lock.Lock()
 	defer p.lock.Unlock()
@@ -67,12 +69,18 @@ func (p *InMemoryPreferences) remove(key string) {
 }
 
 func (p *InMemoryPreferences) fireChange() {
-	p.lock.Lock()
-	defer p.lock.Unlock()
+	p.lock.RLock()
+	defer p.lock.RUnlock()
 
 	for _, l := range p.changeListeners {
-		go l()
+		p.wg.Add(1)
+		go func(listener func()) {
+			defer p.wg.Done()
+			listener()
+		}(l)
 	}
+
+	p.wg.Wait()
 }
 
 // Bool looks up a boolean value for the key
@@ -178,7 +186,8 @@ func (p *InMemoryPreferences) RemoveValue(key string) {
 
 // NewInMemoryPreferences creates a new preferences implementation stored in memory
 func NewInMemoryPreferences() *InMemoryPreferences {
-	p := &InMemoryPreferences{}
-	p.values = make(map[string]interface{})
-	return p
+	return &InMemoryPreferences{
+		values: make(map[string]interface{}),
+		wg:     &sync.WaitGroup{},
+	}
 }