Make ensure_path_nonexistent race condition aware

src/batou/lib/file.py

@@ -5,6 +5,7 @@
 import json
 import os.path
 import pwd
+import random
 import re
 import shutil
 import stat
@@ -19,18 +20,31 @@
 
 
 def ensure_path_nonexistent(path):
+    """Ensure that the given path does not exist.
+
+    strategy: rename the file to a temporary name, then remove it.
+    this allows us to fail in a useful way, since rename is atomic.
+    If we detect that the file exists after the rename, we know that
+    something else is going on and we can bail out.
+    """
+    tmp_nonce = random.getrandbits(32)
+    tmp_filename = path + ".batou-ensure-path-nonexistent-tmp" + str(tmp_nonce)
     if not os.path.lexists(path):
         return
-    if os.path.islink(path):
-        os.unlink(path)
-    elif os.path.isdir(path):
-        shutil.rmtree(path)
+    os.rename(path, tmp_filename)
+    # if the rename did not work, we bail out
+    assert not os.path.lexists(
+        path
+    ), f"{path} should not exist, if it does, it is a race condition."
+    if os.path.islink(tmp_filename):
+        os.unlink(tmp_filename)
+    elif os.path.isdir(tmp_filename):
+        shutil.rmtree(tmp_filename)
     else:
-        os.unlink(path)
+        os.unlink(tmp_filename)
 
 
 class File(Component):
-
     namevar = "path"
 
     ensure = "file"  # or: directory, symlink
@@ -143,13 +157,11 @@ def last_updated(self, key="st_mtime"):
 
 
 class BinaryFile(File):
-
     is_template = False
     encoding = None
 
 
 class Presence(Component):
-
     namevar = "path"
     leading = False
 
@@ -183,7 +195,6 @@ def last_updated(self, key="st_mtime"):
 
 
 class SyncDirectory(Component):
-
     namevar = "path"
     source = None
     exclude = ()
@@ -238,7 +249,6 @@ def namevar_for_breadcrumb(self):
 
 
 class Directory(Component):
-
     namevar = "path"
     leading = False
     source = None
@@ -291,7 +301,6 @@ def namevar_for_breadcrumb(self):
 
 
 class FileComponent(Component):
-
     namevar = "path"
     leading = False
 
@@ -534,7 +543,6 @@ def render(self):
 
 
 class JSONContent(ManagedContentBase):
-
     # Data to be used.
     data = None
 
@@ -569,7 +577,6 @@ def render(self):
 
 
 class YAMLContent(ManagedContentBase):
-
     # Data to be used.
     data = None
 
@@ -588,7 +595,6 @@ def render(self):
 
 
 class Owner(FileComponent):
-
     owner = None
 
     def verify(self):
@@ -603,7 +609,6 @@ def update(self):
 
 
 class Group(FileComponent):
-
     group = None
 
     def verify(self):
@@ -648,7 +653,6 @@ def convert_mode(string: str) -> int:
 
 
 class Mode(FileComponent):
-
     mode = Attribute(default=None)
 
     def configure(self):
@@ -695,7 +699,6 @@ def _select_stat_implementation(self):
 
 
 class Symlink(Component):
-
     namevar = "target"
     source = None
 

src/batou/lib/tests/test_file.py

@@ -74,6 +74,15 @@ def test_ensure_path_does_not_fail_on_nonexisting_path():
     assert not os.path.exists("missing")
 
 
+def test_ensure_path_nonexistent_fails_if_os_rename_is_dummied(tmpdir):
+    os.chdir(str(tmpdir))
+    os.mkdir("dir")
+    with patch("os.rename", Mock(return_value=None)):
+        with pytest.raises(AssertionError) as e:
+            ensure_path_nonexistent("dir")
+    assert "race condition" in str(e.value)
+
+
 def test_presence_creates_nonexisting_file(root):
     p = Presence("path")
     root.component += p