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

Add mutex lock to saveSettings storage call #2737

Merged
merged 1 commit into from Oct 28, 2020
Merged

Conversation

knolleary
Copy link
Member

  • Bugfix (non-breaking change which fixes an issue)

Fixes #2736

When disabling (or enabling) a module that contains lots of individual node sets (ie .js files), the code disables each one in turn - but without any locking to ensure one is done before the next is disabled. Each call to disable causes the settings file to be updated to record the set as being disabled.

This would cause multiple calls to saveSettings that overlap, leading to the error reported in #2736

This fix is to add a mutex lock on saveSettings to ensure it can only have one active call at a time. Applying the fix here will ensure the same symptom can't crop up through other parts of the API that drive rapid changes to the settings file.

There is a separate item to consider - that disabling a whole module causes so many calls to update settings. It would be nice if the calls could be batched up as we know we're about to make lots of updates. The tricky part is how far apart the logic to disable an individual set and the logic to disable the whole module sits. That's for another day.

@knolleary knolleary merged commit c38a490 into master Oct 28, 2020
@knolleary knolleary deleted the fix-async-settings branch October 28, 2020 22:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Error when enabling & disabling large number of nodes.
1 participant