Merge pull request #7352 from nicoddemus/lock-unlink-5456

Suppress errors while removing tmpdir's lock files

changelog/5456.bugfix.rst

@@ -0,0 +1,2 @@
+Fix a possible race condition when trying to remove lock files used to control access to folders
+created by ``tmp_path`` and ``tmpdir``.

src/_pytest/pathlib.py

@@ -1,4 +1,5 @@
 import atexit
+import contextlib
 import fnmatch
 import itertools
 import os
@@ -290,10 +291,14 @@ def ensure_deletable(path: Path, consider_lock_dead_if_created_before: float) ->
         return False
     else:
         if lock_time < consider_lock_dead_if_created_before:
-            lock.unlink()
+            # wa want to ignore any errors while trying to remove the lock such as:
-            return True
+            # - PermissionDenied, like the file permissions have changed since the lock creation
-        else:
+            # - FileNotFoundError, in case another pytest process got here first.
-            return False
+            # and any other cause of failure.
+            with contextlib.suppress(OSError):
+                lock.unlink()
+                return True
+        return False
 
 
 def try_cleanup(path: Path, consider_lock_dead_if_created_before: float) -> None:

testing/test_pathlib.py

@@ -1,9 +1,11 @@
 import os.path
 import sys
+import unittest.mock
 
 import py
 
 import pytest
+from _pytest.pathlib import ensure_deletable
 from _pytest.pathlib import fnmatch_ex
 from _pytest.pathlib import get_extended_length_path_str
 from _pytest.pathlib import get_lock_path
@@ -113,3 +115,22 @@ def test_get_extended_length_path_str():
     assert get_extended_length_path_str(r"\\share\foo") == r"\\?\UNC\share\foo"
     assert get_extended_length_path_str(r"\\?\UNC\share\foo") == r"\\?\UNC\share\foo"
     assert get_extended_length_path_str(r"\\?\c:\foo") == r"\\?\c:\foo"
+
+
+def test_suppress_error_removing_lock(tmp_path):
+    """ensure_deletable should not raise an exception if the lock file cannot be removed (#5456)"""
+    path = tmp_path / "dir"
+    path.mkdir()
+    lock = get_lock_path(path)
+    lock.touch()
+    mtime = lock.stat().st_mtime
+
+    with unittest.mock.patch.object(Path, "unlink", side_effect=OSError):
+        assert not ensure_deletable(
+            path, consider_lock_dead_if_created_before=mtime + 30
+        )
+    assert lock.is_file()
+
+    # check now that we can remove the lock file in normal circumstances
+    assert ensure_deletable(path, consider_lock_dead_if_created_before=mtime + 30)
+    assert not lock.is_file()