Skip to content

Commit

Permalink
Fix a race condition in simultaneous poetry installations updating th…
Browse files Browse the repository at this point in the history
…e 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.
  • Loading branch information
zweger authored Sep 15, 2022
1 parent dbc2030 commit cc3f994
Show file tree
Hide file tree
Showing 2 changed files with 21 additions and 1 deletion.
3 changes: 2 additions & 1 deletion src/poetry/installation/executor.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down
19 changes: 19 additions & 0 deletions src/poetry/utils/helpers.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand Down

0 comments on commit cc3f994

Please sign in to comment.