From cc3f99425206aa2507af6779b1943cfa751206cc Mon Sep 17 00:00:00 2001 From: Zack Weger Date: Thu, 15 Sep 2022 15:47:05 -0400 Subject: [PATCH] Fix a race condition in simultaneous poetry installations updating the cache (#6186) 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. Resolves: #5142 I'm not sure how to test this change, as the conditions which cause this bug to appear are a little hard to reproduce. --- src/poetry/installation/executor.py | 3 ++- src/poetry/utils/helpers.py | 19 +++++++++++++++++++ 2 files changed, 21 insertions(+), 1 deletion(-) diff --git a/src/poetry/installation/executor.py b/src/poetry/installation/executor.py index da0734ed334..b64de0c6848 100644 --- a/src/poetry/installation/executor.py +++ b/src/poetry/installation/executor.py @@ -28,6 +28,7 @@ from poetry.utils._compat import decode from poetry.utils.authenticator import Authenticator from poetry.utils.env import EnvCommandError +from poetry.utils.helpers import atomic_open from poetry.utils.helpers import pluralize from poetry.utils.helpers import remove_directory from poetry.utils.pip import pip_install @@ -698,7 +699,7 @@ def _download_archive(self, operation: Install | Update, link: Link) -> Path: done = 0 archive = self._chef.get_cache_directory_for_link(link) / link.filename archive.parent.mkdir(parents=True, exist_ok=True) - with archive.open("wb") as f: + with atomic_open(archive) as f: for chunk in response.iter_content(chunk_size=4096): if not chunk: break diff --git a/src/poetry/utils/helpers.py b/src/poetry/utils/helpers.py index caee1b279bf..a3f38d14a9b 100644 --- a/src/poetry/utils/helpers.py +++ b/src/poetry/utils/helpers.py @@ -20,6 +20,7 @@ if TYPE_CHECKING: from collections.abc import Callable + from io import BufferedWriter from poetry.core.packages.package import Package from requests import Session @@ -37,6 +38,24 @@ def directory(path: Path) -> Iterator[Path]: os.chdir(cwd) +@contextmanager +def atomic_open(filename: str | os.PathLike[str]) -> Iterator[BufferedWriter]: + """ + write a file to the disk in an atomic fashion + + Taken from requests.utils + (https://github.com/psf/requests/blob/7104ad4b135daab0ed19d8e41bd469874702342b/requests/utils.py#L296) + """ + tmp_descriptor, tmp_name = tempfile.mkstemp(dir=os.path.dirname(filename)) + try: + with os.fdopen(tmp_descriptor, "wb") as tmp_handler: + yield tmp_handler + os.replace(tmp_name, filename) + except BaseException: + os.remove(tmp_name) + raise + + def _on_rm_error(func: Callable[[str], None], path: str, exc_info: Exception) -> None: if not os.path.exists(path): return