From a18874fc59b069ae270feb451a2184d0d90c8123 Mon Sep 17 00:00:00 2001 From: Unrud Date: Thu, 1 Jun 2017 11:21:22 +0200 Subject: [PATCH 1/2] raise exception when locking the storage fails Previously it was silently ignored, which is dangerous when multiple instances of Radicale are running. A configuration option to disable locking was added. --- config | 4 ++++ radicale/config.py | 4 ++++ radicale/storage.py | 31 +++++++++++++++++++++++-------- 3 files changed, 31 insertions(+), 8 deletions(-) diff --git a/config b/config index 9ce8e1f..958fb46 100644 --- a/config +++ b/config @@ -101,6 +101,10 @@ # Folder for storing local collections, created if not present #filesystem_folder = /var/lib/radicale/collections +# Lock the storage. Never start multiple instances of Radicale or edit the +# storage externally while Radicale is running if disabled. +#filesystem_locking = True + # Sync all changes to disk during requests. (This can impair performance.) # Disabling it increases the risk of data loss, when the system crashes or # power fails! diff --git a/radicale/config.py b/radicale/config.py index 03aaffe..3db0e5b 100644 --- a/radicale/config.py +++ b/radicale/config.py @@ -139,6 +139,10 @@ INITIAL_CONFIG = OrderedDict([ "value": "True", "help": "sync all changes to filesystem during requests", "type": bool}), + ("filesystem_locking", { + "value": "True", + "help": "lock the storage while accessing it", + "type": bool}), ("filesystem_close_lock_file", { "value": "False", "help": "close the lock file when no more clients are waiting", diff --git a/radicale/storage.py b/radicale/storage.py index 44ccfd9..8765ff7 100644 --- a/radicale/storage.py +++ b/radicale/storage.py @@ -751,6 +751,10 @@ class Collection(BaseCollection): @classmethod @contextmanager def acquire_lock(cls, mode, user=None): + if not cls.configuration.getboolean("storage", "filesystem_locking"): + yield + return + def condition(): if mode == "r": return not cls._writer @@ -786,21 +790,27 @@ class Collection(BaseCollection): # by arbitrary users try: os.chmod(lock_path, stat.S_IWUSR | stat.S_IRUSR) - except OSError: - cls.logger.debug("Failed to set permissions on lock file") + except OSError as e: + cls.logger.info("Failed to set permissions on lock file:" + " %s", e, exc_info=True) if not cls._lock_file_locked: if os.name == "nt": handle = msvcrt.get_osfhandle(cls._lock_file.fileno()) flags = LOCKFILE_EXCLUSIVE_LOCK if mode == "w" else 0 overlapped = Overlapped() if not lock_file_ex(handle, flags, 0, 1, 0, overlapped): - cls.logger.debug("Locking not supported") + raise RuntimeError("Locking the storage failed: %s" % + ctypes.FormatError()) elif os.name == "posix": _cmd = fcntl.LOCK_EX if mode == "w" else fcntl.LOCK_SH try: fcntl.flock(cls._lock_file.fileno(), _cmd) - except OSError: - cls.logger.debug("Locking not supported") + except OSError as e: + raise RuntimeError("Locking the storage failed: %s" % + e) from e + else: + raise RuntimeError("Locking the storage failed: " + "Unsupported operating system") cls._lock_file_locked = True try: yield @@ -822,12 +832,17 @@ class Collection(BaseCollection): handle = msvcrt.get_osfhandle(cls._lock_file.fileno()) overlapped = Overlapped() if not unlock_file_ex(handle, 0, 1, 0, overlapped): - cls.logger.debug("Unlocking not supported") + raise RuntimeError("Unlocking the storage failed: " + "%s" % ctypes.FormatError()) elif os.name == "posix": try: fcntl.flock(cls._lock_file.fileno(), fcntl.LOCK_UN) - except OSError: - cls.logger.debug("Unlocking not supported") + except OSError as e: + raise RuntimeError("Unlocking the storage failed: " + "%s" % e) from e + else: + raise RuntimeError("Unlocking the storage failed: " + "Unsupported operating system") cls._lock_file_locked = False if cls._waiters: cls._waiters[0].notify() From fd55bbce159bea7c53d85d2049ccae6f04f72281 Mon Sep 17 00:00:00 2001 From: Unrud Date: Thu, 1 Jun 2017 11:37:39 +0200 Subject: [PATCH 2/2] Adjust imports for isort --- radicale/__main__.py | 5 ++--- radicale/tests/test_auth.py | 1 + radicale/tests/test_base.py | 1 + 3 files changed, 4 insertions(+), 3 deletions(-) diff --git a/radicale/__main__.py b/radicale/__main__.py index 111f47b..cd3cf6c 100644 --- a/radicale/__main__.py +++ b/radicale/__main__.py @@ -32,9 +32,8 @@ import ssl import sys from wsgiref.simple_server import make_server -from . import ( - VERSION, Application, RequestHandler, ThreadedHTTPServer, - ThreadedHTTPSServer, config, log) +from . import (VERSION, Application, RequestHandler, ThreadedHTTPServer, + ThreadedHTTPSServer, config, log) def run(): diff --git a/radicale/tests/test_auth.py b/radicale/tests/test_auth.py index dc0fadd..fd3bc6f 100644 --- a/radicale/tests/test_auth.py +++ b/radicale/tests/test_auth.py @@ -26,6 +26,7 @@ import shutil import tempfile import pytest + from radicale import Application, config from .test_base import BaseTest diff --git a/radicale/tests/test_base.py b/radicale/tests/test_base.py index b8ceb84..e18ed17 100644 --- a/radicale/tests/test_base.py +++ b/radicale/tests/test_base.py @@ -26,6 +26,7 @@ import shutil import tempfile import pytest + from radicale import Application, config from . import BaseTest