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

always use lock when writing/reading cache data to/from file #8372

Merged
merged 1 commit into from Aug 11, 2022

Conversation

kkmuffme
Copy link
Contributor

@kkmuffme kkmuffme commented Aug 5, 2022

ran into igbinary end-of-data race condition with PHP 8.1 in a multi-threaded environment in some cases where the file was modified during reading

I just used UnexpectedValueException as that is what was there already in one case. If you prefer another exception, please let me know.

@kkmuffme kkmuffme force-pushed the safely-read-write-cache-data branch 3 times, most recently from fd76799 to 1a94fe2 Compare August 5, 2022 09:19
@kkmuffme
Copy link
Contributor Author

kkmuffme commented Aug 5, 2022

Code review please

@orklah
Copy link
Collaborator

orklah commented Aug 5, 2022

Could we move the method to somewhere common so we could have only one?

@kkmuffme
Copy link
Contributor Author

kkmuffme commented Aug 5, 2022

Yes, if you tell me where you want it :-)

Also which exception do you want then?

@orklah
Copy link
Collaborator

orklah commented Aug 5, 2022

Providers.php seems fine

For the exception, maybe RuntimeException?

@kkmuffme kkmuffme force-pushed the safely-read-write-cache-data branch from 1a94fe2 to 8ca594a Compare August 11, 2022 11:13
@kkmuffme
Copy link
Contributor Author

Done, ready for review again.

There's an internal error with CircleCI that's not from this branch, which is why the test fails:
https://app.circleci.com/pipelines/github/vimeo/psalm/9325/workflows/3ee42730-0f0b-48f7-969a-8ef4a937b743/jobs/34190

@orklah orklah added the release:internal The PR will be included in 'Internal changes' section of the release notes label Aug 11, 2022
@orklah orklah merged commit 7e010a7 into vimeo:4.x Aug 11, 2022
@orklah
Copy link
Collaborator

orklah commented Aug 11, 2022

Thanks!

@kkmuffme kkmuffme deleted the safely-read-write-cache-data branch August 11, 2022 18:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
release:internal The PR will be included in 'Internal changes' section of the release notes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants