diff --git a/tests/test_apk_more_sources.py b/tests/test_apk_more_sources.py index 2cbcf10..80e346f 100644 --- a/tests/test_apk_more_sources.py +++ b/tests/test_apk_more_sources.py @@ -1,4 +1,4 @@ -import hashlib, io, json, os, sys, tempfile, unittest, urllib.error, urllib.request, zipfile +import hashlib, io, json, os, sys, tempfile, time, unittest, urllib.error, urllib.request, zipfile from pathlib import Path from unittest.mock import patch @@ -173,6 +173,30 @@ class PublisherSources(unittest.TestCase): self.assertEqual([p.exists() for p in (oldest, old, kept, recent)], [False, False, True, True]) self.assertEqual([p.exists() for p in (orphan, busy, listing, fresh, tmpdir)], [False, True, False, True, False]) + def test_prune_rechecks_before_deleting_and_spares_apks_in_use(self): + self.root.mkdir(parents=True) + old = time.time() - 7200 + reused, claimed, stale = self.root / 'a.apk', self.root / 'b.apk', self.root / 'c.apk' + for path in (reused, claimed, stale): + path.write_bytes(b'x' * 40) + _web.claim(claimed) + self.addCleanup(_web.release, claimed) + for path in (reused, claimed, stale): + os.utime(str(path), (old, old)) + real = os.listdir + def listdir(folder): + if folder == self.tmp.name: # scanning the second folder: a download reuses a.apk meanwhile + self.assertTrue(_web.touch(reused)) + return real(folder) + with patch.object(_web, 'APK_CAP', 10), patch.object(_web.os, 'listdir', listdir): + _web.prune() + self.assertEqual([p.exists() for p in (reused, claimed, stale)], [True, True, False]) + _web.release(claimed) + with patch.object(_web, 'APK_CAP', 10): + _web.prune() + self.assertFalse(claimed.exists()) + self.assertFalse(_web.touch(stale)) # a pruned APK is reported gone, so it's downloaded again + def test_backoff_honours_retry_after_per_host(self): from apk_sources import SourceLimited from email.utils import formatdate diff --git a/tests/test_apk_search.py b/tests/test_apk_search.py index ec520a9..cecba67 100644 --- a/tests/test_apk_search.py +++ b/tests/test_apk_search.py @@ -33,6 +33,12 @@ class SettingsTest(unittest.TestCase): self.addCleanup(p.stop) for state in (search._running, search._pending, search._status, search._game_data): state.clear() + from apk_sources import _web + self.claims = [] + for name in ('claim', 'release'): # fake downloads aren't real files + p = patch.object(_web, name, side_effect=lambda path, name=name: self.claims.append((name, path))) + p.start() + self.addCleanup(p.stop) class SearchTests(SettingsTest): @@ -204,6 +210,17 @@ class SearchTests(SettingsTest): search.install('one', 'brush') install.assert_not_called() + def test_downloaded_apk_is_protected_from_pruning_while_installing(self): + mod = fake() + def install(apk, **kwargs): + self.assertEqual(self.claims, [('claim', '/fake.apk')]) + raise server.frame_android.FrameError('adb failed') + with patch.object(search, 'modules', return_value=([mod], [])), \ + patch.object(server.frame_android, 'install', install): + with self.assertRaises(server.frame_android.FrameError): + search.install('one', 'brush') + self.assertEqual(self.claims, [('claim', '/fake.apk'), ('release', '/fake.apk')]) + def test_listing_cannot_download(self): mod = fake() mod.details = lambda s, i: dict(ENTRIES[0], downloadable=False) diff --git a/ui/apk_sources/_web.py b/ui/apk_sources/_web.py index 6f83838..d2f6604 100644 --- a/ui/apk_sources/_web.py +++ b/ui/apk_sources/_web.py @@ -7,6 +7,36 @@ from . import SourceError, SourceLimited UA = 'FrameControl/0.1' APK_CAP = 2 * 1024 ** 3 # cached APKs across all sources, least recently used go first +RECENT = 3600 # an APK used this recently may be about to be installed; never pruned +_use_lock = threading.Lock() # pruning's final check and delete vs touch()/claim() +_in_use = {} # APK path -> installs using it + + +def touch(path): + """Mark a cached APK as just used; False if pruning already removed it.""" + with _use_lock: + try: + os.utime(str(path)) + return True + except FileNotFoundError: + return False + + +def claim(path): + """Protect a downloaded APK from pruning until release(path).""" + path = os.path.abspath(str(path)) + with _use_lock: + os.utime(path) # raises if it has gone + _in_use[path] = _in_use.get(path, 0) + 1 + + +def release(path): + path = os.path.abspath(str(path)) + with _use_lock: + if _in_use.get(path, 0) > 1: + _in_use[path] -= 1 + else: + _in_use.pop(path, None) BACKOFF = 600 # seconds to leave a host alone after 403/429 without Retry-After _limited = {} # host -> time.time() before which we don't contact it _limited_lock = threading.Lock() @@ -77,16 +107,17 @@ def prune(): except OSError: pass total = sum(size for _, size, _ in apks) - for mtime, size, path in sorted(apks): + for _, size, path in sorted(apks): if total <= APK_CAP: break - if now - mtime < 3600: # may be about to be installed - continue - try: - os.remove(path) - total -= size - except OSError: - pass + with _use_lock: # the scan is old news: check again right before deleting + try: + if os.path.abspath(path) in _in_use or time.time() - os.stat(path).st_mtime < RECENT: + continue + os.remove(path) + total -= size + except OSError: + pass def checked_url(url, hosts): diff --git a/ui/apk_sources/fdroid.py b/ui/apk_sources/fdroid.py index 689d3c2..d6dcbb1 100644 --- a/ui/apk_sources/fdroid.py +++ b/ui/apk_sources/fdroid.py @@ -647,9 +647,7 @@ def download(source, entry_id, version_code=None): sha = version['sha256'] path = frame_host.cache_dir('apk-sources', sha + '.apk') try: - if path.exists() and _sha256(path) == sha: - os.utime(str(path)) # most recently used, for cache pruning - else: + if not (path.exists() and _sha256(path) == sha and _web.touch(path)): # touch: recently used, not pruned path.parent.mkdir(parents=True, exist_ok=True) fd, tmp = tempfile.mkstemp(dir=str(path.parent), suffix='.part') os.close(fd) diff --git a/ui/apk_sources/search.py b/ui/apk_sources/search.py index c8fd6d0..324fee5 100644 --- a/ui/apk_sources/search.py +++ b/ui/apk_sources/search.py @@ -314,7 +314,15 @@ def install(source_id, entry_id, version_code=None, progress=None): kwargs['artwork'] = downloaded.get('artwork') or entry.get('artwork') if progress: progress('Installing', None) - result = frame_android.install(downloaded['apk'], **kwargs) + from apk_sources import _web + try: + _web.claim(downloaded['apk']) # no cache pruning while it installs + except OSError as e: + raise SourceError('The downloaded APK disappeared before installing; try again') from e + try: + result = frame_android.install(downloaded['apk'], **kwargs) + finally: + _web.release(downloaded['apk']) if obb: # OBB files go into the app's own instance, which only exists while the app runs. with _lock: