Skip to content

Commit

Permalink
Wait for goroutines to finish when firing a change to an app preferen…
Browse files Browse the repository at this point in the history
…ces (#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
  • Loading branch information
s77rt authored and andydotxyz committed Jul 7, 2021
1 parent 31dff03 commit bd856c1
Showing 1 changed file with 15 additions and 6 deletions.
21 changes: 15 additions & 6 deletions internal/preferences.go
Expand Up @@ -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()
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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{},
}
}

0 comments on commit bd856c1

Please sign in to comment.