From 2aafcd5df547f8d009b6d2e197a517906ad37c8a Mon Sep 17 00:00:00 2001 From: Unrud Date: Fri, 23 Oct 2020 21:28:19 +0200 Subject: [PATCH] Use renameat2 on Linux for atomic exchanging of files --- radicale/pathutils.py | 58 +++++++++++++++++++ .../multifilesystem/create_collection.py | 10 ++-- 2 files changed, 62 insertions(+), 6 deletions(-) diff --git a/radicale/pathutils.py b/radicale/pathutils.py index 6a2cb1b..400a604 100644 --- a/radicale/pathutils.py +++ b/radicale/pathutils.py @@ -24,7 +24,9 @@ Helper functions for working with the file system. import contextlib import os import posixpath +import sys import threading +from tempfile import TemporaryDirectory if os.name == "nt": import ctypes @@ -66,6 +68,23 @@ if os.name == "nt": elif os.name == "posix": import fcntl +HAVE_RENAMEAT2 = False +if sys.platform == "linux": + import ctypes + + RENAME_EXCHANGE = 2 + try: + renameat2 = ctypes.CDLL(None, use_errno=True).renameat2 + except AttributeError: + pass + else: + HAVE_RENAMEAT2 = True + renameat2.argtypes = [ + ctypes.c_int, ctypes.c_char_p, + ctypes.c_int, ctypes.c_char_p, + ctypes.c_uint] + renameat2.restype = ctypes.c_int + class RwLock: """A readers-Writer lock that locks a file.""" @@ -127,6 +146,45 @@ class RwLock: self._writer = False +def rename_exchange(src, dst): + """Exchange the files or directories `src` and `dst`. + + Both `src` and `dst` must exist but may be of different types. + + On Linux with renameat2 the operation is atomic. + On other platforms it's not atomic. + + """ + src_dir, src_base = os.path.split(src) + dst_dir, dst_base = os.path.split(dst) + src_dir = src_dir or os.curdir + dst_dir = dst_dir or os.curdir + if not src_base or not dst_base: + raise ValueError("Invalid arguments: %r -> %r" % (src, dst)) + if HAVE_RENAMEAT2: + src_base_bytes = os.fsencode(src_base) + dst_base_bytes = os.fsencode(dst_base) + src_dir_fd = os.open(src_dir, 0) + try: + dst_dir_fd = os.open(dst_dir, 0) + try: + if renameat2(src_dir_fd, src_base_bytes, + dst_dir_fd, dst_base_bytes, + RENAME_EXCHANGE) != 0: + errno = ctypes.get_errno() + raise OSError(errno, os.strerror(errno)) + finally: + os.close(dst_dir_fd) + finally: + os.close(src_dir_fd) + else: + with TemporaryDirectory( + prefix=".Radicale.tmp-", dir=src_dir) as tmp_dir: + os.rename(dst, os.path.join(tmp_dir, "interim")) + os.rename(src, dst) + os.rename(os.path.join(tmp_dir, "interim"), src) + + def fsync(fd): if os.name == "posix" and hasattr(fcntl, "F_FULLFSYNC"): fcntl.fcntl(fd, fcntl.F_FULLFSYNC) diff --git a/radicale/storage/multifilesystem/create_collection.py b/radicale/storage/multifilesystem/create_collection.py index 6be6345..fe0290e 100644 --- a/radicale/storage/multifilesystem/create_collection.py +++ b/radicale/storage/multifilesystem/create_collection.py @@ -55,12 +55,10 @@ class StorageCreateCollectionMixin: elif props.get("tag") == "VADDRESSBOOK": col._upload_all_nonatomic(items, suffix=".vcf") - # This operation is not atomic on the filesystem level but it's - # very unlikely that one rename operations succeeds while the - # other fails or that only one gets written to disk. - if os.path.exists(filesystem_path): - os.rename(filesystem_path, os.path.join(tmp_dir, "delete")) - os.rename(tmp_filesystem_path, filesystem_path) + if os.path.lexists(filesystem_path): + pathutils.rename_exchange(tmp_filesystem_path, filesystem_path) + else: + os.rename(tmp_filesystem_path, filesystem_path) self._sync_directory(parent_dir) return self._collection_class(