From b33a8f04c3c2a39cbb7b7d55efdf05cb79ff04ef Mon Sep 17 00:00:00 2001 From: Andy Williams Date: Wed, 8 Sep 2021 23:18:25 +0100 Subject: [PATCH] Fix accidental concurrent preference writes Fixes #2449 --- app/preferences.go | 22 +++++++++++----------- app/preferences_test.go | 20 ++++++++++++++++++++ 2 files changed, 31 insertions(+), 11 deletions(-) diff --git a/app/preferences.go b/app/preferences.go index 0859dbb2ad..d5f303fa41 100644 --- a/app/preferences.go +++ b/app/preferences.go @@ -5,7 +5,6 @@ import ( "os" "path/filepath" "sync" - "time" "fyne.io/fyne/v2" "fyne.io/fyne/v2/internal" @@ -23,15 +22,6 @@ type preferences struct { // Declare conformity with Preferences interface var _ fyne.Preferences = (*preferences)(nil) -func (p *preferences) resetIgnore() { - go func() { - time.Sleep(time.Millisecond * 100) // writes are not always atomic. 10ms worked, 100 is safer. - p.prefLock.Lock() - p.ignoreChange = false - p.prefLock.Unlock() - }() -} - func (p *preferences) save() error { return p.saveToFile(p.storagePath()) } @@ -40,7 +30,11 @@ func (p *preferences) saveToFile(path string) error { p.prefLock.Lock() p.ignoreChange = true p.prefLock.Unlock() - defer p.resetIgnore() + defer func() { + p.prefLock.Lock() + p.ignoreChange = false + p.prefLock.Unlock() + }() err := os.MkdirAll(filepath.Dir(path), 0700) if err != nil { // this is not an exists error according to docs return err @@ -56,11 +50,17 @@ func (p *preferences) saveToFile(path string) error { return err } } + defer file.Close() encode := json.NewEncoder(file) p.InMemoryPreferences.ReadValues(func(values map[string]interface{}) { err = encode.Encode(&values) }) + + err2 := file.Sync() + if err == nil { + err = err2 + } return err } diff --git a/app/preferences_test.go b/app/preferences_test.go index 5900901c7e..6df17177d4 100644 --- a/app/preferences_test.go +++ b/app/preferences_test.go @@ -40,6 +40,26 @@ func TestPreferences_Save(t *testing.T) { assert.JSONEq(t, string(expected), string(content)) } +func TestPreferences_Save_OverwriteFast(t *testing.T) { + p := loadPreferences("dummy") + p.WriteValues(func(val map[string]interface{}) { + val["key"] = "value" + }) + + path := filepath.Join(os.TempDir(), "fynePrefs2.json") + defer os.Remove(path) + p.saveToFile(path) + + p.WriteValues(func(val map[string]interface{}) { + val["key2"] = "value2" + }) + p.saveToFile(path) + + p2 := loadPreferences("dummy") + p2.loadFromFile(path) + assert.Equal(t, "value2", p2.String("key2")) +} + func TestPreferences_Load(t *testing.T) { p := loadPreferences("dummy") p.loadFromFile(filepath.Join("testdata", "preferences.json"))