Merge pull request #6312 from hexagonrecursion/bp-unlink

Backport:  safe_unlink (was: truncate_and_unlink)

src/borg/cache.py

@@ -25,7 +25,7 @@ from .helpers import yes, hostname_is_unique
 from .helpers import remove_surrogates
 from .helpers import ProgressIndicatorPercent, ProgressIndicatorMessage
 from .helpers import set_ec, EXIT_WARNING
-from .helpers import truncate_and_unlink
+from .helpers import safe_unlink
 from .helpers import msgpack
 from .item import ArchiveItem, ChunkListEntry
 from .crypto.key import PlaintextKey
@@ -745,7 +745,7 @@ class LocalCache(CacheStatsMixin):
                                                   filename=bin_to_hex(archive_id) + '.compact') as fd:
                     chunk_idx.write(fd)
             except Exception:
-                truncate_and_unlink(fn_tmp)
+                safe_unlink(fn_tmp)
             else:
                 os.rename(fn_tmp, fn)
 

src/borg/helpers.py

@@ -2399,12 +2399,12 @@ def secure_erase(path):
     os.unlink(path)
 
 
-def truncate_and_unlink(path):
+def safe_unlink(path):
     """
-    Truncate and then unlink *path*.
+    Safely unlink (delete) *path*.
 
-    Do not create *path* if it does not exist.
-    Open *path* for truncation in r+b mode (=O_RDWR|O_BINARY).
+    If we run out of space while deleting the file, we try truncating it first.
+    BUT we truncate only if path is the only hardlink referring to this content.
 
     Use this when deleting potentially large files when recovering
     from a VFS error such as ENOSPC. It can help a full file system
@@ -2412,13 +2412,27 @@ def truncate_and_unlink(path):
     in repository.py for further explanations.
     """
     try:
-        with open(path, 'r+b') as fd:
-            fd.truncate()
-    except OSError as err:
-        if err.errno != errno.ENOTSUP:
+        os.unlink(path)
+    except OSError as unlink_err:
+        if unlink_err.errno != errno.ENOSPC:
+            # not free space related, give up here.
             raise
-        # don't crash if the above ops are not supported.
-    os.unlink(path)
+        # we ran out of space while trying to delete the file.
+        st = os.stat(path)
+        if st.st_nlink > 1:
+            # rather give up here than cause collateral damage to the other hardlink.
+            raise
+        # no other hardlink! try to recover free space by truncating this file.
+        try:
+            # Do not create *path* if it does not exist, open for truncation in r+b mode (=O_RDWR|O_BINARY).
+            with open(path, 'r+b') as fd:
+                fd.truncate()
+        except OSError:
+            # truncate didn't work, so we still have the original unlink issue - give up:
+            raise unlink_err
+        else:
+            # successfully truncated the file, try again deleting it:
+            os.unlink(path)
 
 
 def popen_with_error_handling(cmd_line: str, log_prefix='', **kwargs):

src/borg/platform/base.py

@@ -3,7 +3,7 @@ import os
 import socket
 import uuid
 
-from borg.helpers import truncate_and_unlink
+from borg.helpers import safe_unlink
 
 """
 platform base module
@@ -168,7 +168,7 @@ class SaveFile:
     def __enter__(self):
         from .. import platform
         try:
-            truncate_and_unlink(self.tmppath)
+            safe_unlink(self.tmppath)
         except FileNotFoundError:
             pass
         self.fd = platform.SyncFile(self.tmppath, self.binary)
@@ -178,7 +178,7 @@ class SaveFile:
         from .. import platform
         self.fd.close()
         if exc_type is not None:
-            truncate_and_unlink(self.tmppath)
+            safe_unlink(self.tmppath)
             return
         os.replace(self.tmppath, self.path)
         platform.sync_dir(os.path.dirname(self.path))

src/borg/remote.py

@@ -27,7 +27,7 @@ from .helpers import hostname_is_unique
 from .helpers import replace_placeholders
 from .helpers import sysinfo
 from .helpers import format_file_size
-from .helpers import truncate_and_unlink
+from .helpers import safe_unlink
 from .helpers import prepare_subprocess_env
 from .logger import create_logger, setup_logging
 from .helpers import msgpack
@@ -1144,7 +1144,7 @@ class RepositoryCache(RepositoryNoCache):
                 fd.write(packed)
         except OSError as os_error:
             try:
-                truncate_and_unlink(file)
+                safe_unlink(file)
             except FileNotFoundError:
                 pass  # open() could have failed as well
             if os_error.errno == errno.ENOSPC:

src/borg/repository.py

@@ -19,7 +19,7 @@ from .helpers import Location
 from .helpers import ProgressIndicatorPercent
 from .helpers import bin_to_hex
 from .helpers import hostname_is_unique
-from .helpers import secure_erase, truncate_and_unlink
+from .helpers import secure_erase, safe_unlink
 from .helpers import msgpack
 from .locking import Lock, LockError, LockErrorT
 from .logger import create_logger
@@ -1294,7 +1294,7 @@ class LoggedIO:
             if segment > transaction_id:
                 if segment in self.fds:
                     del self.fds[segment]
-                truncate_and_unlink(filename)
+                safe_unlink(filename)
                 count += 1
             else:
                 break
@@ -1402,7 +1402,7 @@ class LoggedIO:
         if segment in self.fds:
             del self.fds[segment]
         try:
-            truncate_and_unlink(self.segment_filename(segment))
+            safe_unlink(self.segment_filename(segment))
         except FileNotFoundError:
             pass
 

src/borg/testsuite/helpers.py

@@ -1,3 +1,4 @@
+import errno
 import hashlib
 import io
 import os
@@ -27,6 +28,7 @@ from ..helpers import chunkit
 from ..helpers import safe_ns, safe_s, SUPPORT_32BIT_PLATFORMS
 from ..helpers import popen_with_error_handling
 from ..helpers import dash_open
+from ..helpers import safe_unlink
 
 from . import BaseTestCase, FakeInputs
 
@@ -999,3 +1001,32 @@ def test_dash_open():
     assert dash_open('-', 'w') is sys.stdout
     assert dash_open('-', 'rb') is sys.stdin.buffer
     assert dash_open('-', 'wb') is sys.stdout.buffer
+
+
+def test_safe_unlink_is_safe(tmpdir):
+    contents = b"Hello, world\n"
+    victim = tmpdir / 'victim'
+    victim.write_binary(contents)
+    hard_link = tmpdir / 'hardlink'
+    hard_link.mklinkto(victim)
+
+    safe_unlink(str(hard_link))
+
+    assert victim.read_binary() == contents
+
+
+def test_safe_unlink_is_safe_ENOSPC(tmpdir, monkeypatch):
+    contents = b"Hello, world\n"
+    victim = tmpdir / 'victim'
+    victim.write_binary(contents)
+    hard_link = tmpdir / 'hardlink'
+    hard_link.mklinkto(victim)
+
+    def os_unlink(_):
+        raise OSError(errno.ENOSPC, "Pretend that we ran out of space")
+    monkeypatch.setattr(os, "unlink", os_unlink)
+
+    with pytest.raises(OSError):
+        safe_unlink(str(hard_link))
+
+    assert victim.read_binary() == contents