F-Droid: one load lock per repository; downloads never hold the settings lock

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>

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

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)
+        current = next((s for s in settings['repos'] if s['id'] == source['id']), None)
-        if existing:
+        if current and current['fingerprint'] != source['fingerprint']:
-            if pin and pin != existing['fingerprint']:
+            raise SourceError('repository already has a different pinned fingerprint; remove it first')
-                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
         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):