Avoid race-condition when reading localeCollection

[christianharrington]

The previous implementation was an example of a check-then-act race-condition: There is no guarantee that the cached value still exists when it is subsequently read out.

This would cause errors later when $this->localeCollection could suddenly have the value false.

See #3188 for more context.

Checklist

• Code follows the PSR-2 coding style
• Tests have been created, run and adjusted as needed
• The PR is created against the lowest maintained branch
• Reviewer - PR Title is brief but complete and starts with FEATURE|TASK|BUGFIX
• Reviewer - The first section explains the change briefly for change-logs
• Reviewer - Breaking Changes are marked with !!! and have upgrade-instructions

[kdambekalns]

Seems edge-casey to me, but totally makes sense.

[kdambekalns]

Just the tests need to be adjusted…

[christianharrington]

Seems edge-casey to me, but totally makes sense.

This was happening many times a day in production for us before we implemented a mitigation 🤷🏻

Just the tests need to be adjusted…

Yeah, sorry, I had difficulty figuring out how to run them locally. I couldn't find any documentation on it, so I ended up reverse-engineering it from the Github Actions workflow.

I found the error raised in the test quite confusing as well:

Expectation failed for method name is "addLocale" when invoked 4 time(s).

The issue was in fact the method name being used for the mocked cache 🤔

[markusguenther]

Thanks for bringing that up again! Sorry I missed it yesterday with the tests and was too quick with the merge since it worked locally. 😅

Thanks for your contribution, and it’s great to hear that you still use Flow at famly.

[kdambekalns]

This was happening many times a day in production for us before we implemented a mitigation

Doh. Ok, to me it seems that cache should exist rather reliably… 🙈