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

Delete config cache only after the cache file was written #14754

Merged
merged 1 commit into from Aug 16, 2019

Conversation

tsteur
Copy link
Member

@tsteur tsteur commented Aug 12, 2019

I noticed we deleted the config cache in IniFileChain when we dumpChanges() but the actual cache file is not yet overwritten. This actually happens in the config class. We need to make sure to delete the config cache only after we have written the config file. Otherwise there could be race conditions where the outdated config gets cached again, and if the user made two changes within one hour (that's how long the cache is active), then a previous change would be potentially undone.

I was going to use the Core.configFileChanged event to listen to config file changes, however, at the time I would listen to the event using Piwik::addAction('Core.configFileChanged', ...) the DI container might not exist yet etc. Therefore I figured it's best to invalidate the cache specifically by calling the delete method within the config to ensure it'll work and not cause any trouble to something so critical.

@tsteur tsteur added not-in-changelog For issues or pull requests that should not be included in our release changelog on matomo.org. Needs Review PRs that need a code review labels Aug 12, 2019
@tsteur tsteur added this to the 3.12.0 milestone Aug 12, 2019
@diosmosis diosmosis merged commit 1b7e84e into 3.x-dev Aug 16, 2019
@diosmosis diosmosis deleted the configcachefix branch August 16, 2019 01:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Needs Review PRs that need a code review not-in-changelog For issues or pull requests that should not be included in our release changelog on matomo.org.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants