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

use lock to fix race condition in cache #8240

Merged

Conversation

kkmuffme
Copy link
Contributor

When running multiple psalm instances in the same directory (e.g. I want to analyze 2 distinct directories or X files only) at the same time (e.g. CI pipeline), I was getting random "Unexpected value when loading from file content hashes" errors due to race conditions

By using a read/write lock with wait this issue is resolved.
The "write" lock success is not checked, since this issue only happens with simultaneous runs which means the data of multiple writes at the same time the file is locked is the same that is currently being written.

@kkmuffme kkmuffme force-pushed the fix-cache-race-condition-multiple-psalm-instances branch 2 times, most recently from bd5f051 to c60ef5f Compare July 10, 2022 07:33
@kkmuffme
Copy link
Contributor Author

This is ready to be reviewed & merged

return [];
}

// file_get_contents ignores any locks and gets the file, but we have the lock on $fp anyway
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why not use fgets then? file_get_contents would have to reopen a stream, no?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Changed to fread now

@kkmuffme kkmuffme force-pushed the fix-cache-race-condition-multiple-psalm-instances branch from c60ef5f to 7742d8a Compare July 10, 2022 08:03
@orklah orklah added release:fix The PR will be included in 'Fixes' section of the release notes labels Jul 10, 2022
@orklah orklah merged commit ce7d4ee into vimeo:4.x Jul 10, 2022
@orklah
Copy link
Collaborator

orklah commented Jul 10, 2022

Thanks!

@kkmuffme kkmuffme deleted the fix-cache-race-condition-multiple-psalm-instances branch July 10, 2022 09:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
release:fix The PR will be included in 'Fixes' section of the release notes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants