From 328b7ed211b72728f6f0ee3bb559c10462769648 Mon Sep 17 00:00:00 2001 From: saphid <4596216+saphid@users.noreply.github.com> Date: Mon, 28 Sep 2026 22:06:51 +1000 Subject: [PATCH] F-Droid: one load lock per repository; downloads never hold the settings lock Co-Authored-By: Claude Opus 5.5 (1M context) --- tests/test_fdroid_sources.py | 26 ++++++++++++++++++++++++++ ui/apk_sources/fdroid.py | 36 +++++++++++++++++++++++------------- 2 files changed, 49 insertions(+), 13 deletions(-) diff --git a/tests/test_fdroid_sources.py b/tests/test_fdroid_sources.py index 50cab33..04069bb 100644 --- a/tests/test_fdroid_sources.py +++ b/tests/test_fdroid_sources.py @@ -229,6 +229,32 @@ class Repositories(unittest.TestCase): self.assertIn('images', fdroid.search(source, 'example')[0]) self.assertEqual(self.fetch_mock.call_count, 4) + def test_slow_download_blocks_neither_settings_nor_other_repos(self): + import threading + source = self.add() + other = dict(source, id='other-repo') + started, release = threading.Event(), threading.Event() + def fetch(url, path, maximum): + if threading.current_thread().name == 'slow': + started.set() + release.wait(5) + self.fetch(url, path, maximum) + self.fetch_mock.side_effect = fetch + slow = threading.Thread(target=fdroid._load, args=(other, True), name='slow') + slow.start() + try: + self.assertTrue(started.wait(2)) + results = [] + # The settings lock is free and another repo still loads while this one downloads. + check = threading.Thread(target=lambda: results.append( + (fdroid.set_enabled('fdroid', False), len(fdroid._load(source, force=True)[0])))) + check.start() + check.join(2) + self.assertEqual(results, [(None, 1)]) + finally: + release.set() + slow.join() + def test_cached_index_does_not_cross_pins(self): source = self.add() source['fingerprint'] = '0' * 64 diff --git a/ui/apk_sources/fdroid.py b/ui/apk_sources/fdroid.py index 7063a7f..f182a1e 100644 --- a/ui/apk_sources/fdroid.py +++ b/ui/apk_sources/fdroid.py @@ -25,7 +25,8 @@ from frame_catalog import _IndexReader, _reduce_index, _sha256 KIND = 'fdroid' CACHE_VERSION = 2 -_LOCK = threading.RLock() +_LOCK = threading.RLock() # settings only; never held while downloading +_load_locks = {} # Published by the repository operators; a user repository without a pin uses TOFU. FDROID_PIN = '43238d512c1e5eb2d6569f4a3afbf5523418b82e0a3ed1552770abb9a9c9ccab' IZZY_PIN = '3bf0d6abfeae2f401707b6d966be743bf0eee49c2561b9ba39073711f628937a' @@ -382,12 +383,17 @@ def _v1(content, path): path.write_text(json.dumps({'packages': packages})) +def _source_lock(source_id): + with _LOCK: + return _load_locks.setdefault(source_id, threading.Lock()) + + def _load(source, force=False): if not re.fullmatch(r'[a-z0-9-]+', source['id']): raise SourceError('invalid source id') _url(source['url'], source.get('fingerprint')) cache = frame_host.cache_dir('apk-sources', source['id'] + '.json') - with _LOCK: + with _source_lock(source['id']): if not force and cache.exists() and time.time() - cache.stat().st_mtime < 86400: try: saved = json.loads(cache.read_text()) @@ -425,21 +431,25 @@ def _load(source, force=False): def add_repo(url, fingerprint=None, name=None): url, pin = _url(url, fingerprint) + with _LOCK: + existing = next((s for s in _read()['repos'] if s['url'] == url), None) + if existing: + if pin and pin != existing['fingerprint']: + raise SourceError('repository already has a different pinned fingerprint; remove it first') + pin = existing['fingerprint'] + source = dict(id='fdroid-user-' + hashlib.sha256(url.encode()).hexdigest()[:20], kind=KIND, + name=name or (existing or {}).get('name') or urllib.parse.urlsplit(url).hostname, + url=url, builtin=False, enabled=True, trust='user', fingerprint=pin) + _, source['fingerprint'] = _load(source, force=True) # network work outside the settings lock + source['trust_on_first_use'] = existing.get('trust_on_first_use', False) if existing else pin is None with _LOCK: settings = _read() - existing = next((s for s in settings['repos'] if s['url'] == url), None) - if existing: - if pin and pin != existing['fingerprint']: - raise SourceError('repository already has a different pinned fingerprint; remove it first') - pin = existing['fingerprint'] - source = dict(id='fdroid-user-' + hashlib.sha256(url.encode()).hexdigest()[:20], kind=KIND, - name=name or (existing or {}).get('name') or urllib.parse.urlsplit(url).hostname, - url=url, builtin=False, enabled=True, trust='user', fingerprint=pin) - _, source['fingerprint'] = _load(source, force=True) - source['trust_on_first_use'] = existing.get('trust_on_first_use', False) if existing else pin is None + current = next((s for s in settings['repos'] if s['id'] == source['id']), None) + if current and current['fingerprint'] != source['fingerprint']: + raise SourceError('repository already has a different pinned fingerprint; remove it first') settings['repos'] = [s for s in settings['repos'] if s['id'] != source['id']] + [source] _write(_storage(), settings) - return source + return source def remove_repo(source_id):