Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fyne_settings has a race condition when saving+reloading settings (seen on Windows) #1165

Closed
MartyMacGyver opened this issue Jul 4, 2020 · 2 comments
Labels
bug Something isn't working
Milestone

Comments

@MartyMacGyver
Copy link

Describe the bug:

This may or may not be Windows-specific.

When fyne_settings is executed and a setting is changed, the change is written to the settings.json file, which triggers a WRITE event twice via fsnotify. Under certain conditions that will trigger a Fyne error: Settings load error.

To Reproduce:

Steps to reproduce the behaviour:

  1. cd \fyne\cmd\fyne_settings
  2. go install . && fyne_settings
  3. Change a setting and click Apply
  4. The error will appear in the output window

Screenshots:

n/a

Example code:

Seen in Fyne v1.3.0

Device (please complete the following information):

  • OS: Windows 10 Pro x64
  • Version: Build 2004
  • Go version: 1.14.4 x64
  • Fyne version: v1.3.0
@MartyMacGyver
Copy link
Author

MartyMacGyver commented Jul 4, 2020

The reason this is happening (again, at least on Windows (NTFS to be even more exact), as the filesystem semantics may be different on other systems) appears to be related to how watchFile()'s callback works and what's actually happening in saveToFile().

In saveToFile() (triggered by pressing Apply), Create() is called, which truncates the file if it exists. This causes a filesystem write, which will eventually be detected by fsnotify's watcher, triggering the callback above.

Then, Encode() writes the updated information to the file, again triggering the same callback.

The result is that fileChanged() (with its load() call) gets called twice in quick succession during each save. It appears one of the load() calls is being made while the save() write is still in progress.

If you add something that takes a bit of time in fileChanged() before s.load(), the problem goes away. A brief sleep (time.Sleep(100 * time.Millisecond)) or even a Println() is enough to ensure the watcher doesn't load the file before the parameter save is complete. This is not a best practice of course - it would be better to do something more deterministic (perhaps via a semaphore) to ensure a save() is not in progress during load(), at least within the same application.

@andydotxyz andydotxyz added the bug Something isn't working label Jul 4, 2020
@andydotxyz andydotxyz added this to the 1.3.x milestone Jul 4, 2020
andydotxyz added a commit that referenced this issue Jul 21, 2020
This could happen as settings file writes, but we get another event on completion.

Fixes #1165
@andydotxyz
Copy link
Member

Thanks, fix is on develop for testing

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants