From b25855d13a3accd950e71c76de2c6b5c9772f4d7 Mon Sep 17 00:00:00 2001 From: saphid <4596216+saphid@users.noreply.github.com> Date: Thu, 8 Oct 2026 19:41:38 +1100 Subject: [PATCH 1/3] Server: leave without interpreter teardown after a clean stop (SIGTERM segfault) On Linux with Python 3.13 (GitHub's ubuntu-latest runner, CPython 3.13.16), about 1 stop in 100 crashed the server with SIGSEGV. A native backtrace taken at the crash shows background threads inside OpenSSL (X509_STORE_set_default_paths_ex, called from _ssl while the app-index download threads build their TLS context) while the main thread was already in exit(), where libcrypto's own atexit cleanup frees the state those threads are using. After the shutdown cleanup has run, the server now flushes stdout/stderr and calls os._exit(0). Daemon threads (index downloads, stdin watcher, telemetry, contact, headset link, request handlers) cannot be stopped promptly, and nothing in the server registers atexit work; the OS frees the one-server lock with the process. The stop test now runs its scenario five times with faulthandler on. Co-Authored-By: Claude Opus 5.5 (1M context) --- tests/test_server.py | 31 ++++++++++++++++++++++--------- ui/server.py | 10 ++++++++++ 2 files changed, 32 insertions(+), 9 deletions(-) diff --git a/tests/test_server.py b/tests/test_server.py index b82034c..aa3da36 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,27 @@ 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) class ArtworkSettings(unittest.TestCase): diff --git a/ui/server.py b/ui/server.py index 66108cd..ee7ab91 100755 --- a/ui/server.py +++ b/ui/server.py @@ -2742,6 +2742,16 @@ def main(): if proc.poll() is None: proc.terminate() _purge_titles(now=float("inf")) # unconfirmed title uploads + # 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) + # after a stop signal. Nothing here registers atexit work, and the OS frees + # the one-server lock with the process. + sys.stdout.flush() + sys.stderr.flush() + os._exit(0) if __name__ == "__main__": From db9bee2903a0f5a38cb53d811e5fce1995142359 Mon Sep 17 00:00:00 2001 From: saphid <4596216+saphid@users.noreply.github.com> Date: Thu, 8 Oct 2026 19:58:44 +1100 Subject: [PATCH 2/3] Server: clear in-flight staging folders on the way out, and a dead server's at start Leaving through os._exit skips the weakref finalizers that clean up a TemporaryDirectory still in use by a background thread, so quitting during a patched VR APK transfer left the APK behind (and likewise a file staged for the assistant, or a half-downloaded app index in the cache folder). Those folders now carry the server's PID (frame-vr-, frame-agent-, and .download- in the apk-sources cache folder), like the web-install and title staging folders already did. sweep_tmp covers all five; it still runs at start for dead servers' folders, and with own=True at the end of the shutdown cleanup for this server's. The exit comment now says which exit work is skipped and why that is safe. Tests: the sweep test covers every prefix and own=True; a new server test stops a server with in-flight folders and checks they are gone, and that a dead server's are removed at the next start. Co-Authored-By: Claude Opus 5.5 (1M context) --- tests/test_server.py | 42 ++++++++++++++++++++++++++++++++++++++++ tests/test_webinstall.py | 33 ++++++++++++++++++++----------- ui/apk_sources/fdroid.py | 3 ++- ui/frame_agent.py | 4 +++- ui/frame_android.py | 3 ++- ui/server.py | 42 +++++++++++++++++++++++++++++----------- 6 files changed, 102 insertions(+), 25 deletions(-) diff --git a/tests/test_server.py b/tests/test_server.py index aa3da36..b7efee9 100644 --- a/tests/test_server.py +++ b/tests/test_server.py @@ -310,6 +310,48 @@ class OneServer(unittest.TestCase): 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 + + 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)] + 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): """The settings panel's endpoints, with and without the page's X-Frame-UI key.""" 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 0f0364f..2d178d9 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' @@ -167,7 +168,7 @@ def install(apk_path, flatscreen=None, name=None, source=None, icon_png=None, xr add = xr_compat_files(apk_path) if (info['vr'] if xr_compat is None else xr_compat) else {} 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') info['patched'] = patch(apk_path, patched, add)['patched'] info['launchable'] = True diff --git a/ui/server.py b/ui/server.py index ee7ab91..deada68 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 @@ -1922,23 +1923,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 @@ -2742,13 +2756,19 @@ 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) - # after a stop signal. Nothing here registers atexit work, and the OS frees - # the one-server lock with the process. + # 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) From 6e566db1a7eee8ba17fdac16cd07b79a7a007356 Mon Sep 17 00:00:00 2001 From: saphid <4596216+saphid@users.noreply.github.com> Date: Thu, 8 Oct 2026 20:07:09 +1100 Subject: [PATCH 3/3] Test: stop the server the way each platform really does On Windows, terminate() is TerminateProcess (a hard kill, exit 1, no cleanup); the app stops the server there by closing stdin. The staging-folder test now stops it by closing stdin on every platform, and also by SIGTERM off Windows. Co-Authored-By: Claude Opus 5.5 (1M context) --- tests/test_server.py | 42 +++++++++++++++++++++++++----------------- 1 file changed, 25 insertions(+), 17 deletions(-) diff --git a/tests/test_server.py b/tests/test_server.py index b7efee9..8309f88 100644 --- a/tests/test_server.py +++ b/tests/test_server.py @@ -334,23 +334,31 @@ class OneServer(unittest.TestCase): (d / "app.apk").write_bytes(b"\0" * 4096) return d - 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)] - 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() + # 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):