diff --git a/tests/test_server.py b/tests/test_server.py index b82034c..8309f88 100644 --- a/tests/test_server.py +++ b/tests/test_server.py @@ -10,6 +10,7 @@ import http.client import io import json import os +import shutil import socket import struct import subprocess @@ -287,15 +288,77 @@ class OneServer(unittest.TestCase): @unittest.skipIf(os.name == "nt", "no SIGTERM on Windows") def test_sigterm_while_the_app_holds_stdin_exits_cleanly(self): - """The app keeps stdin open; a stop signal used to abort Python (SIGABRT) at exit.""" - env = {**os.environ, "FRAME_CONTROL_DATA_DIR": tempfile.mkdtemp(prefix="frame-one-server-"), - "FRAME_ALIAS": "frame-control-test.invalid"} - proc = subprocess.Popen([sys.executable, str(ROOT / "ui" / "server.py"), "--port", "0", "--exit-on-eof"], - env=env, stdin=subprocess.PIPE, stdout=subprocess.PIPE, stderr=subprocess.STDOUT, text=True) - self.addCleanup(lambda: (proc.stdin.close(), proc.stdout.close())) - self.assertIn("Frame Control on", proc.stdout.readline()) - proc.terminate() - self.assertEqual(proc.wait(30), 0, proc.stdout.read()) + """The app keeps stdin open; a stop signal used to abort Python (SIGABRT) at exit, + and later, now and then, crash it (SIGSEGV) while background threads were still + loading TLS certificates. Run a few times: that crash came about 1 run in 100.""" + for attempt in range(5): + with self.subTest(attempt=attempt): + env = {**os.environ, "FRAME_CONTROL_DATA_DIR": tempfile.mkdtemp(prefix="frame-one-server-"), + "FRAME_ALIAS": "frame-control-test.invalid", "PYTHONFAULTHANDLER": "1"} + proc = subprocess.Popen([sys.executable, str(ROOT / "ui" / "server.py"), "--port", "0", "--exit-on-eof"], + env=env, stdin=subprocess.PIPE, stdout=subprocess.PIPE, + stderr=subprocess.STDOUT, text=True) + try: + self.assertIn("Frame Control on", proc.stdout.readline()) + proc.terminate() + self.assertEqual(proc.wait(30), 0, proc.stdout.read()) + finally: + if proc.poll() is None: + proc.kill() + proc.wait() + proc.stdin.close() + proc.stdout.close() + shutil.rmtree(env["FRAME_CONTROL_DATA_DIR"], ignore_errors=True) + + def test_staging_folders_cleared_when_stopped_mid_transfer(self): + """The server leaves without interpreter teardown, so TemporaryDirectory cleanup + doesn't run for work still in progress: its own staging folders go on the way out, + and a dead server's at the next start.""" + dead = subprocess.Popen([sys.executable, "-c", "pass"]) + dead.wait() + home = tempfile.mkdtemp(prefix="frame-stop-home-") + self.addCleanup(shutil.rmtree, home, ignore_errors=True) + env = {**os.environ, "FRAME_CONTROL_DATA_DIR": os.path.join(home, "data"), + "FRAME_ALIAS": "frame-control-test.invalid", "HOME": home, "XDG_CACHE_HOME": os.path.join(home, ".cache"), + "LOCALAPPDATA": home, "APPDATA": home} + index_dir = Path(subprocess.run( + [sys.executable, "-c", "import sys; sys.path.insert(0, sys.argv[1]); import frame_host; " + "print(frame_host.cache_dir('apk-sources'))", str(ROOT / "ui")], + env=env, capture_output=True, text=True, check=True).stdout.strip()) + index_dir.mkdir(parents=True) + tmp = Path(tempfile.gettempdir()) + + def staged(folder, prefix, pid): + d = Path(tempfile.mkdtemp(prefix=f"{prefix}{pid}-", dir=folder)) + self.addCleanup(shutil.rmtree, d, ignore_errors=True) + (d / "app.apk").write_bytes(b"\0" * 4096) + return d + + # Stopped as the app stops it: by closing stdin (the only way on Windows, + # where terminate() is a hard kill) and, elsewhere, by SIGTERM too. + for stop in ["stdin"] + (["sigterm"] if os.name != "nt" else []): + with self.subTest(stop=stop): + left_by_dead = [staged(tmp, "frame-vr-", dead.pid), staged(index_dir, ".download-", dead.pid)] + proc = subprocess.Popen([sys.executable, str(ROOT / "ui" / "server.py"), "--port", "0", "--exit-on-eof"], + env=env, stdin=subprocess.PIPE, stdout=subprocess.PIPE, + stderr=subprocess.STDOUT, text=True) + try: + self.assertIn("Frame Control on", proc.stdout.readline()) + self.assertEqual([d for d in left_by_dead if d.exists()], []) + in_flight = [staged(tmp, "frame-vr-", proc.pid), staged(tmp, "frame-agent-", proc.pid), + staged(index_dir, ".download-", proc.pid)] + if stop == "stdin": + proc.stdin.close() + else: + proc.terminate() + self.assertEqual(proc.wait(30), 0, proc.stdout.read()) + self.assertEqual([d for d in in_flight if d.exists()], []) + finally: + if proc.poll() is None: + proc.kill() + proc.wait() + proc.stdin.close() + proc.stdout.close() class ArtworkSettings(unittest.TestCase): diff --git a/tests/test_webinstall.py b/tests/test_webinstall.py index e197b49..ccbb560 100644 --- a/tests/test_webinstall.py +++ b/tests/test_webinstall.py @@ -386,17 +386,28 @@ class ServerJobs(unittest.TestCase): def test_dead_servers_leftovers_swept(self): dead = subprocess.Popen([sys.executable, "-c", "pass"]) dead.wait() - # Downloads and title staging (unzipped titles) are both swept. - for prefix in (self.server.WEB_TMP_PREFIX, self.server.frame_titles.TMP_PREFIX): - gone = tempfile.mkdtemp(prefix=f"{prefix}{dead.pid}-") - live = tempfile.mkdtemp(prefix=f"{prefix}{os.getpid()}-") - try: - self.server.sweep_tmp() - self.assertFalse(os.path.exists(gone), prefix) - self.assertTrue(os.path.exists(live), prefix) - finally: - shutil.rmtree(gone, ignore_errors=True) - shutil.rmtree(live, ignore_errors=True) + s = self.server + with tempfile.TemporaryDirectory() as cache, \ + mock.patch.object(s.frame_host, "cache_dir", lambda *parts: Path(cache).joinpath(*parts)): + # Downloads, title staging, patched VR APKs, files staged for the + # assistant, and app-index downloads (in the cache folder). + places = s._tmp_places() + self.assertEqual({p for _, p in places}, {s.WEB_TMP_PREFIX, s.frame_titles.TMP_PREFIX, + s.frame_android.TMP_PREFIX, s.frame_agent.TMP_PREFIX, + s.apk_fdroid.TMP_PREFIX}) + for folder, prefix in places: + folder.mkdir(parents=True, exist_ok=True) + gone = tempfile.mkdtemp(prefix=f"{prefix}{dead.pid}-", dir=folder) + live = tempfile.mkdtemp(prefix=f"{prefix}{os.getpid()}-", dir=folder) + try: + s.sweep_tmp() + self.assertFalse(os.path.exists(gone), prefix) + self.assertTrue(os.path.exists(live), prefix) + s.sweep_tmp(own=True) # on the way out: this server's own go too + self.assertFalse(os.path.exists(live), prefix) + finally: + shutil.rmtree(gone, ignore_errors=True) + shutil.rmtree(live, ignore_errors=True) def test_temp_dir_failure_ends_the_job(self): job, dispatch = self.run_job(mkdtemp_error=OSError("disk full")) diff --git a/ui/apk_sources/fdroid.py b/ui/apk_sources/fdroid.py index 0006dba..ff02cb4 100644 --- a/ui/apk_sources/fdroid.py +++ b/ui/apk_sources/fdroid.py @@ -27,6 +27,7 @@ from frame_catalog import _IndexReader, _reduce_index, _sha256 KIND = 'fdroid' CACHE_VERSION = 2 +TMP_PREFIX = '.download-' # in the cache folder, then the server's PID, so server.sweep_tmp can clear a stopped run's _LOCK = threading.RLock() # settings only; never held while downloading _load_locks = {} _refreshing = {} # source id -> background refresh thread @@ -530,7 +531,7 @@ def _load(source, force=False): return found[:2] cache.parent.mkdir(parents=True, exist_ok=True) try: - with tempfile.TemporaryDirectory(dir=str(cache.parent)) as tmp: + with tempfile.TemporaryDirectory(prefix=f'{TMP_PREFIX}{os.getpid()}-', dir=str(cache.parent)) as tmp: jar, raw = Path(tmp) / 'index.jar', Path(tmp) / 'index.json' v2 = True try: diff --git a/ui/frame_agent.py b/ui/frame_agent.py index b8a3dbe..40b003d 100644 --- a/ui/frame_agent.py +++ b/ui/frame_agent.py @@ -9,6 +9,8 @@ import subprocess import threading import time +TMP_PREFIX = 'frame-agent-' # then the server's PID, so server.sweep_tmp can clear a stopped run's + class Approvals: def __init__(self): @@ -109,7 +111,7 @@ def call(server, body): if name == 'send_file': # Stage the reviewed bytes before the existing transfer helper reads them. import tempfile - with tempfile.TemporaryDirectory(prefix='frame-agent-') as tmp: + with tempfile.TemporaryDirectory(prefix=f'{TMP_PREFIX}{os.getpid()}-') as tmp: source = Path(action['arguments']['path']) with source.open('rb') as stream: data = stream.read(16 * 1024**2 + 1) diff --git a/ui/frame_android.py b/ui/frame_android.py index 4802c0b..8872e3c 100644 --- a/ui/frame_android.py +++ b/ui/frame_android.py @@ -23,6 +23,7 @@ from frame_apk_sign import repack ROOT = os.path.dirname(os.path.dirname(os.path.abspath(__file__))) FRAME = os.environ.get('FRAME_ALIAS', 'frame') +TMP_PREFIX = 'frame-vr-' # then the server's PID, so server.sweep_tmp can clear a stopped run's patched APK APPS_DIR = 'Applications/Android' # relative to the Frame's $HOME COMPAT = '.local/share/Steam/steamapps/compatdata' SHADERS = '.local/share/Steam/steamapps/shadercache' @@ -200,7 +201,7 @@ def install(apk_path, flatscreen=None, name=None, source=None, icon_png=None, xr info['xr_layer_missing'] = True with _install_lock: if add or info['repairable']: - with tempfile.TemporaryDirectory(prefix='frame-vr-') as tmp: + with tempfile.TemporaryDirectory(prefix=f'{TMP_PREFIX}{os.getpid()}-') as tmp: patched = os.path.join(tmp, 'app.apk') try: info['patched'] = patch(apk_path, patched, add)['patched'] diff --git a/ui/server.py b/ui/server.py index f71b7d8..6aaa394 100755 --- a/ui/server.py +++ b/ui/server.py @@ -45,6 +45,7 @@ import frame_agent # noqa: E402 import frame_assistant # noqa: E402 import frame_android # noqa: E402 from apk_sources import search as apk_search, SourceError # noqa: E402 +from apk_sources import fdroid as apk_fdroid # noqa: E402 import frame_apk_versions # noqa: E402 import frame_catalog # noqa: E402 import frame_devices # noqa: E402 @@ -1935,23 +1936,36 @@ def _pid_alive(pid): return True -def sweep_tmp(): - """Delete download and title staging folders left by a server killed mid-install. +def _tmp_places(): + """Where each kind of staging folder goes, and its name before the server's PID.""" + tmp = Path(tempfile.gettempdir()) + return [(tmp, WEB_TMP_PREFIX), (tmp, frame_titles.TMP_PREFIX), (tmp, frame_android.TMP_PREFIX), + (tmp, frame_agent.TMP_PREFIX), (frame_host.cache_dir("apk-sources"), apk_fdroid.TMP_PREFIX)] - Folders carry the server's PID, so only a dead server's are taken. + +def sweep_tmp(own=False): + """Delete staging folders (downloads, unzipped titles, patched APKs, staged files, + app-index downloads) left by a server that stopped mid-way. + + Folders carry the server's PID, so only a dead server's are taken; own=True (on + the way out, see main) takes this server's too. """ - for prefix in (WEB_TMP_PREFIX, frame_titles.TMP_PREFIX): - for d in Path(tempfile.gettempdir()).glob(f"{prefix}*"): - _sweep_one(prefix, d) + for folder, prefix in _tmp_places(): + try: + found = list(folder.glob(f"{prefix}*")) + except OSError: + continue + for d in found: + _sweep_one(prefix, d, own) -def _sweep_one(prefix, d): +def _sweep_one(prefix, d, own=False): m = re.fullmatch(re.escape(prefix) + r"(\d+)-.*", d.name) if not m: return pid = int(m[1]) try: - if pid != os.getpid() and not _pid_alive(pid) and d.is_dir(): + if (own if pid == os.getpid() else not _pid_alive(pid)) and d.is_dir(): shutil.rmtree(d, ignore_errors=True) except OSError: pass @@ -2757,6 +2771,22 @@ def main(): if proc.poll() is None: proc.terminate() _purge_titles(now=float("inf")) # unconfirmed title uploads + # Staging folders of work still running (a patched APK mid-copy, an index + # download): leaving below skips the cleanup their TemporaryDirectory would + # get at interpreter exit. sweep_tmp at the next start catches any missed. + sweep_tmp(own=True) + # Stopped as asked, and everything above is cleaned up. Leave now, without + # Python's interpreter teardown: daemon threads are still running (app index + # downloads, the stdin watcher, telemetry, the headset link, request handlers) + # and none can be stopped promptly. Tearing the interpreter down under them + # occasionally crashed the process (SIGSEGV, seen on Python 3.13 on Linux): + # OpenSSL's exit cleanup freed state those threads were using. That teardown + # also runs atexit handlers and weakref finalizers; nothing here registers + # atexit work, the only finalizers are TemporaryDirectory cleanups (swept + # above), and the OS frees the one-server lock with the process. + sys.stdout.flush() + sys.stderr.flush() + os._exit(0) if __name__ == "__main__":