From d8e9d67c7ac3d25645881e18daeac673e378dee8 Mon Sep 17 00:00:00 2001 From: Thomas Waldmann Date: Wed, 19 Aug 2026 23:12:54 +0200 Subject: [PATCH] --bypass-lock: also skip the cache lock, fixes #7255 The security checks done for commands without cache access only read the cache config, so they do not need the exclusive cache lock. Previously, e.g. "borg list --bypass-lock" was blocked by a concurrently running "borg create" using the same $HOME, because the security checks always acquired the cache lock. Commands that really open the cache (e.g. "borg info", "borg config --cache") still acquire the cache lock, as they might write to the cache. Co-Authored-By: Claude Fable 5 --- src/borg/archiver.py | 3 ++- src/borg/cache.py | 17 ++++++++++------- src/borg/testsuite/archiver.py | 22 ++++++++++++++++++++-- 3 files changed, 32 insertions(+), 10 deletions(-) diff --git a/src/borg/archiver.py b/src/borg/archiver.py index 1ed0b4ab5d..44243a28b2 100644 --- a/src/borg/archiver.py +++ b/src/borg/archiver.py @@ -193,7 +193,8 @@ def wrapper(self, args, **kwargs): if 'compression' in args: kwargs['key'].compressor = args.compression.compressor if secure: - assert_secure(repository, kwargs['manifest'], self.lock_wait) + # --bypass-lock also skips the cache lock, we only read the cache config here, see #7255 + assert_secure(repository, kwargs['manifest'], self.lock_wait, lock=lock) if cache: with Cache(repository, kwargs['key'], kwargs['manifest'], progress=getattr(args, 'progress', False), lock_wait=self.lock_wait, diff --git a/src/borg/cache.py b/src/borg/cache.py index 54874ecc81..9f87435808 100644 --- a/src/borg/cache.py +++ b/src/borg/cache.py @@ -130,7 +130,8 @@ def assert_location_matches(self, cache_config=None): logger.debug('security: updating location stored in cache and security dir') with SaveFile(self.location_file) as fd: fd.write(repository_location) - if cache_config: + if cache_config and cache_config.lock is not None: + # only write to the cache config if we hold its lock, see #7255 cache_config.save() def assert_no_manifest_replay(self, manifest, key, cache_config=None): @@ -161,14 +162,14 @@ def assert_key_type(self, key, cache_config=None): if self.known() and not self.key_matches(key): raise Cache.EncryptionMethodMismatch() - def assert_secure(self, manifest, key, *, cache_config=None, warn_if_unencrypted=True, lock_wait=None): + def assert_secure(self, manifest, key, *, cache_config=None, warn_if_unencrypted=True, lock_wait=None, lock=True): # warn_if_unencrypted=False is only used for initializing a new repository. # Thus, avoiding asking about a repository that's currently initializing. self.assert_access_unknown(warn_if_unencrypted, manifest, key) if cache_config: self._assert_secure(manifest, key, cache_config) else: - cache_config = CacheConfig(self.repository, lock_wait=lock_wait) + cache_config = CacheConfig(self.repository, lock_wait=lock_wait, lock=lock) if cache_config.exists(): with cache_config: self._assert_secure(manifest, key, cache_config) @@ -203,9 +204,9 @@ def assert_access_unknown(self, warn_if_unencrypted, manifest, key): raise Cache.CacheInitAbortedError() -def assert_secure(repository, manifest, lock_wait): +def assert_secure(repository, manifest, lock_wait, lock=True): sm = SecurityManager(repository) - sm.assert_secure(manifest, manifest.key, lock_wait=lock_wait) + sm.assert_secure(manifest, manifest.key, lock_wait=lock_wait, lock=lock) def recanonicalize_relative_location(cache_location, repository): @@ -238,12 +239,13 @@ def discover_files_cache_name(path): class CacheConfig: - def __init__(self, repository, path=None, lock_wait=None): + def __init__(self, repository, path=None, lock_wait=None, lock=True): self.repository = repository self.path = cache_dir(repository, path) self.config_path = os.path.join(self.path, 'config') self.lock = None self.lock_wait = lock_wait + self.do_lock = lock def __enter__(self): self.open() @@ -268,7 +270,8 @@ def create(self): config.write(fd) def open(self): - self.lock = Lock(os.path.join(self.path, 'lock'), exclusive=True, timeout=self.lock_wait).acquire() + if self.do_lock: + self.lock = Lock(os.path.join(self.path, 'lock'), exclusive=True, timeout=self.lock_wait).acquire() self.load() def load(self): diff --git a/src/borg/testsuite/archiver.py b/src/borg/testsuite/archiver.py index 884b9ee125..13501d5cfc 100644 --- a/src/borg/testsuite/archiver.py +++ b/src/borg/testsuite/archiver.py @@ -40,7 +40,7 @@ from ..crypto.keymanager import RepoIdMismatch, NotABorgKeyFile from ..crypto.file_integrity import FileIntegrityError from ..hashindex import ChunkIndex -from ..helpers import Location, get_security_dir +from ..helpers import Location, get_cache_dir, get_security_dir from ..helpers import Manifest, MandatoryFeatureUnsupported, ArchiveInfo from ..helpers import init_ec_warnings from ..helpers import EXIT_SUCCESS, EXIT_WARNING, EXIT_ERROR, Error, CancelledByUser, RTError, CommandError @@ -52,7 +52,7 @@ from ..nanorst import RstToTextLazy, rst_to_terminal from ..patterns import IECommand, PatternMatcher, parse_pattern from ..item import Item, ItemDiff, chunks_contents_equal -from ..locking import LockFailed +from ..locking import Lock, LockFailed, LockTimeout from ..logger import setup_logging from ..remote import RemoteRepository, PathNotAllowed from ..repository import Repository @@ -2119,6 +2119,24 @@ def test_readonly_mount(self): with self.fuse_mount(self.repository_location, None, '--bypass-lock'): pass + def test_bypass_lock_locked_cache(self): + # --bypass-lock shall not be blocked by the cache lock another borg process holds, see #7255 + self.cmd('init', '--encryption=repokey', self.repository_location) + self.create_src_archive('test') + with Repository(self.repository_path) as repository: + cache_path = os.path.join(get_cache_dir(), repository.id_str) + host, pid, tid = platform.get_process_id() + with Lock(os.path.join(cache_path, 'lock'), exclusive=True, id=(host, pid, tid + 1)): + # verify that the cache lock normally blocks other borg processes + if self.FORK_DEFAULT: + self.cmd('list', self.repository_location, exit_code=EXIT_ERROR) + else: + with pytest.raises(LockTimeout): + self.cmd('list', self.repository_location) + # verify that command works despite the locked cache when using --bypass-lock + output = self.cmd('list', self.repository_location, '--bypass-lock') + self.assert_in('test', output) + @pytest.mark.skipif('BORG_TESTS_IGNORE_MODES' in os.environ, reason='modes unreliable') def test_umask(self): self.create_regular_file('file1', size=1024 * 80)