Fix a race condition in simultaneous poetry installations updating the cache

[zweger]

If one poetry installation is writing to the cache while a second installer is attempting to read that cache file, the second installation will fail because the in-flight cache file is invalid while it is still being written to by the first process.

This PR resolves this issue by having Poetry write to a temporary file in the cache directory first, and then rename the file after it's written, which is ~atomic.

Pull Request Check List

Resolves: #5142

• Added tests for changed code.
• Updated documentation for changed code.

I'm not sure how to test this change, as the conditions which cause this bug to appear are a little hard to reproduce.

[radoering]

Makes sense to me. I just wonder if it's wise to import a helper function from requests.utils. It's probably not part of the public API, is it? Maybe, we should add "our own" atomic_open() (to poetry.utils.helpers maybe)?

I don't know how to test this either. Maybe, we can go without a test here.

@neersighted What do you think?

[neersighted]

I'm wondering if #6471 is the better solution -- I agree that we should yoink atomic_open into this repo (with a comment for attribution/licensing) if we go forward with this. However, as either would likely be part of 1.3 exclusively, I think making the OS work for us instead of against us is the better long-term path forward.

[dimbleby]

#6471 and this are independent fixes, it would make perfect sense to merge them both

(#6471 relates to files owned by cachecontrol's filecache, this one relates to a file written by poetry regardless of whether caching is enabled or not)

Edit: rather than the race condition with multiple poetry processes that zweger describes, I bet a more common bug that this fixes is: user hits Ctrl-C while part way through a download. Currently this leaves a half-downloaded file in place, if I understand correctly this should stop that from happening.

[neersighted]

Fair enough, and good point

[zweger]

Hi, thanks for the feedback. I've updated this PR to include atomic_open in poetry.utils.helpers. Regarding testing, I will note that while poetry doesn't have a test for this particular situation (concurrent installations / ctrl+c during install), poetry's current tests do at least cover this code.

[github-actions]

This pull request has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.