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

handle race condition when creation and deletion of a numbered dir overlap #4222

Merged
merged 1 commit into from Oct 26, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
1 change: 1 addition & 0 deletions changelog/4181.bugfix.rst
@@ -0,0 +1 @@
Handle race condition between creation and deletion of temporary folders.
14 changes: 10 additions & 4 deletions src/_pytest/pathlib.py
Expand Up @@ -182,9 +182,15 @@ def cleanup_on_exit(lock_path=lock_path, original_pid=pid):
return register(cleanup_on_exit)


def delete_a_numbered_dir(path):
"""removes a numbered directory"""
create_cleanup_lock(path)
def maybe_delete_a_numbered_dir(path):
"""removes a numbered directory if its lock can be obtained"""
try:
create_cleanup_lock(path)
except (OSError, EnvironmentError):
# known races:
# * other process did a cleanup at the same time
# * deletable folder was found
return
parent = path.parent

garbage = parent.joinpath("garbage-{}".format(uuid.uuid4()))
Expand Down Expand Up @@ -214,7 +220,7 @@ def ensure_deletable(path, consider_lock_dead_if_created_before):
def try_cleanup(path, consider_lock_dead_if_created_before):
"""tries to cleanup a folder if we can ensure its deletable"""
if ensure_deletable(path, consider_lock_dead_if_created_before):
delete_a_numbered_dir(path)
maybe_delete_a_numbered_dir(path)


def cleanup_candidates(root, prefix, keep):
Expand Down
9 changes: 8 additions & 1 deletion testing/test_tmpdir.py
Expand Up @@ -4,6 +4,7 @@
import six

import pytest
from _pytest import pathlib
from _pytest.pathlib import Path


Expand Down Expand Up @@ -284,11 +285,17 @@ def test_rmtree(self, tmp_path):
rmtree(adir, force=True)
assert not adir.exists()

def test_cleanup_symlink(self, tmp_path):
def test_cleanup_ignores_symlink(self, tmp_path):
the_symlink = tmp_path / (self.PREFIX + "current")
attempt_symlink_to(the_symlink, tmp_path / (self.PREFIX + "5"))
self._do_cleanup(tmp_path)

def test_removal_accepts_lock(self, tmp_path):
folder = pathlib.make_numbered_dir(root=tmp_path, prefix=self.PREFIX)
pathlib.create_cleanup_lock(folder)
pathlib.maybe_delete_a_numbered_dir(folder)
assert folder.is_dir()


def attempt_symlink_to(path, to_path):
"""Try to make a symlink from "path" to "to_path", skipping in case this platform
Expand Down