You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
Repository navigation
The new SeparateBodyFileCache design makes updates/reads non-atomic #324
Program crashes after writing metadata but before writing body. Now the cache thinks the body is empty.
Scenario 2
Multiple processes using the same cache directory. Process A writes metadata, but not yet body. Process B gets metadata, then gets body, and now it thinks the body is empty.
Workarounds
For pip I added logic that basically pretends a cache entry is missing if it doesn't have both body and metadata files.
Solutions
Probably need both a new API that involves writing and reading both at once, and an implementation that makes sure that's atomic. See also the somewhat related #325.
Probably need both a new API that involves writing and reading both at once, and an implementation that makes sure that's atomic. See also the somewhat related #325.
This makes sense to me -- if I'm understanding correctly, this would mean replacing get + get_body with a single get, correct? Along with internal changes to ensure that we synchronize/atomize the two I/O operations?
For cross-file atomicity, writing the body file first might do the trick, if reading code reads the metadata file first. This would allow the file format to stay the same, at least.
(Would also be nice if write API didn't take the full bytes, since that means you have to keep full response in memory, but that maybe a bit too much work. Or could use the mmap hack again to work around that and accepts buffer API-implementing objects.)
Note that a combination of fixing this and #235 in the ways suggested in previous comment, plus os.replace(), might allow removing the need for locking altogether.
pip doesn't use locking because historic reliance on deprecated lockfile. They'd vendor the newer filelock if need be, but in general seem happier with fewer dependencies since they need to vendor everything.
Activity
itamarst commentedon Oct 18, 2023
This is my fault, by the way... 😢
We discussed various solutions on the pip side here: pypa/pip#12361
woodruffw commentedon Oct 18, 2023
Thanks for the report!
This makes sense to me -- if I'm understanding correctly, this would mean replacing
get+get_bodywith a singleget, correct? Along with internal changes to ensure that we synchronize/atomize the two I/O operations?itamarst commentedon Oct 18, 2023
Yeah, plus have a single write API function.
For cross-file atomicity, writing the body file first might do the trick, if reading code reads the metadata file first. This would allow the file format to stay the same, at least.
(Would also be nice if write API didn't take the full bytes, since that means you have to keep full response in memory, but that maybe a bit too much work. Or could use the mmap hack again to work around that and accepts buffer API-implementing objects.)
itamarst commentedon Oct 18, 2023
Note that a combination of fixing this and #235 in the ways suggested in previous comment, plus
os.replace(), might allow removing the need for locking altogether.pipdoesn't use locking because historic reliance on deprecatedlockfile. They'd vendor the newerfilelockif need be, but in general seem happier with fewer dependencies since they need to vendor everything.