From 97ff3df51d7dbd5efbf14f4a7083d0a74af7f898 Mon Sep 17 00:00:00 2001 From: zero Date: Sun, 25 Oct 2026 12:56:52 -0600 Subject: [PATCH 1/3] fix(launcher): supervise the guest session and clean up after it The Riot Client does not exit when its window closes, and a loader that dies first leaves launchd/mldr and the client tree holding memory until the runtime is restarted by hand. run-riot-client.sh now execs a small supervisor instead of the guest loader: - SIGINT/SIGTERM/SIGHUP are forwarded to the guest process group, the loader gets a grace period, and darling shutdown runs before any leftover guest process is stopped. - Leftovers are stopped only when they carry this prefix's __mldr_sockpath and the same uid, deepest-first, SIGTERM then SIGKILL. The guest PID namespace is learned while the session runs; a namespace equal to the host's is refused. - Cleanup only runs when this launcher started the runtime, so a second launch that attaches to an existing server leaves it running. - RCLOL_STOP_AFTER_CLIENT_SECONDS=N adds an opt-in idle stop: it fires only after a managed client window was seen and is gone for N seconds with no game process, and never when the window state cannot be read. Off by default. - RCLOL_CLEAN_ON_EXIT=0 keeps the previous exec-only behaviour. Adds 23 unit tests for attribution, namespace scoping, shutdown ordering, the attached-session guard and the idle decision. --- docs/TROUBLESHOOTING.md | 39 ++- tests/test_session_supervisor.py | 262 ++++++++++++++++++ tools/run-riot-client.sh | 10 +- tools/session-supervisor.py | 442 +++++++++++++++++++++++++++++++ 4 files changed, 751 insertions(+), 2 deletions(-) create mode 100644 tests/test_session_supervisor.py create mode 100755 tools/session-supervisor.py diff --git a/docs/TROUBLESHOOTING.md b/docs/TROUBLESHOOTING.md index f4a5488..ea5d55f 100644 --- a/docs/TROUBLESHOOTING.md +++ b/docs/TROUBLESHOOTING.md @@ -184,6 +184,43 @@ is available from the configured tools directory: RCLOL_CLEAN_STALE_HELPERS=1 python3 build/runtime/tools/clean-stale-helpers.py --dry-run ``` +## Launcher stays attached after closing the client + +`scripts/run.sh` keeps a supervisor between the terminal and Darling. It exists +because the Riot Client does not exit when its window closes, and because a +loader that dies before `darlingserver` can leave launchd/mldr and its client +tree running. The supervisor: + +- forwards `Ctrl+C`, terminal close and `SIGTERM` to the loader, then shuts the + runtime down and stops anything still inside the guest PID namespace. A + process is only stopped when it carries this prefix's `__mldr_sockpath` and + the same user, and a namespace equal to the host's is refused, so another + prefix or a host process is never touched. +- shuts the runtime down after the loader exits normally. If a second launch + attached to a runtime that was already running, that runtime is left alone. +- leaves everything as it was with `RCLOL_CLEAN_ON_EXIT=0`, which is the old + `exec` behaviour. + +Closing the client window is not the same as quitting the client. The window is +gone but `RiotClientServices`, `LeagueClientUx` and their CEF helpers stay +resident, and `xwininfo` can still list their windows. If closing the window +should end the session, opt in to an idle stop: + +```sh +RCLOL_STOP_AFTER_CLIENT_SECONDS=120 ./scripts/run.sh +``` + +The session stops after no managed League/Riot window and no game process have +been seen for that many seconds, but only after a client window was seen. It +never stops while a game runs, and it never stops when the window state cannot +be read (for example a missing display or `xprop`), so a Wayland session without +Xwayland still has to quit the client normally. The default is off. + +Quit the Riot Client itself (tray icon, then Quit) to end the session in every +setup. If the launcher was killed with `SIGKILL`, or the whole terminal was +terminated without a signal reaching the supervisor, the runtime still has to +be stopped with `darling shutdown` as described below. + ### Client window stays black after a match The League client keeps its Chromium/CEF UI (`LeagueClientUx` and its helpers) @@ -465,7 +502,7 @@ It prints each thread-local access that calls its thunk without the descriptor i ## Base install refuses to continue while Darling is not running -`sudo cmake --install` prints `Darling currently running for , shutting it down...` several times and then `Darling launchd PID … remains alive after shutdown`. A guest launchd outlived its darlingserver, so `darling shutdown` sees no container to stop. Automatic cleanup by process name is disabled because guest names and UIDs do not prove prefix ownership (killing by name risks terminating an active launchd from an unrelated prefix or session). If setup is blocked, shut down the original Darling session from its own namespace (`darling shutdown`), or inspect the leftover PID under `/proc/` before terminating it manually. +`sudo cmake --install` prints `Darling currently running for , shutting it down...` several times and then `Darling launchd PID … remains alive after shutdown`. A guest launchd outlived its darlingserver, so `darling shutdown` sees no container to stop. Automatic cleanup by process name is disabled because guest names and UIDs do not prove prefix ownership (killing by name risks terminating an active launchd from an unrelated prefix or session). The launcher supervisor stops such leftovers when the session ends through `Ctrl+C`, a closed terminal or a normal client exit. If setup is already blocked, shut down the original Darling session from its own namespace (`darling shutdown`), or inspect the leftover PID under `/proc/` before terminating it manually. ## Lower fight FPS than Windows diff --git a/tests/test_session_supervisor.py b/tests/test_session_supervisor.py new file mode 100644 index 0000000..476e4d0 --- /dev/null +++ b/tests/test_session_supervisor.py @@ -0,0 +1,262 @@ +"""Session supervisor: attribution, idle decisions and scoped cleanup.""" +import importlib.util +import io +import os +from pathlib import Path +import tempfile +import unittest + +ROOT = Path(__file__).resolve().parents[1] +spec = importlib.util.spec_from_file_location( + 'session_supervisor', ROOT / 'tools/session-supervisor.py') +supervisor = importlib.util.module_from_spec(spec) +spec.loader.exec_module(supervisor) + + +def fake_process(root, pid, ppid=1, state='S', comm='mldr', environ=b'', ns=None, + uid=None): + proc = root / str(pid) + (proc / 'ns').mkdir(parents=True) + fields = [state, str(ppid)] + ['0'] * 17 + ['100'] + (proc / 'stat').write_text('%d (%s) %s\n' % (pid, comm, ' '.join(fields))) + (proc / 'comm').write_text(comm + '\n') + (proc / 'cmdline').write_bytes(comm.encode() + b'\0') + if environ is not None: + (proc / 'environ').write_bytes(environ) + if ns is not None: + os.symlink(ns, proc / 'ns/pid') + return proc + + +class AttributionTests(unittest.TestCase): + def setUp(self): + self.temp = tempfile.TemporaryDirectory() + self.addCleanup(self.temp.cleanup) + self.root = Path(self.temp.name) + self.prefix = self.root / '.darling' + self.socket = self.prefix / '.darlingserver.sock' + self.ns = 'pid:[4026532000]' + + def process(self, pid, **kwargs): + return fake_process(self.root, pid, ns=kwargs.pop('ns', self.ns), **kwargs) + + def test_socket_names_the_owner_prefix(self): + proc = self.process(10, comm='RiotClientServices', + environ=b'__mldr_sockpath=%s\0' % os.fsencode(self.socket)) + self.assertEqual(supervisor.socket_prefix(proc), self.prefix.resolve()) + self.assertEqual(supervisor.prefix_namespace(self.prefix, self.root), self.ns) + + def test_another_prefix_is_not_owned(self): + proc = self.process(10, comm='RiotClientServices', + environ=b'__mldr_sockpath=/elsewhere/.darlingserver.sock\0') + self.assertFalse(supervisor.owned_by_prefix(proc, self.prefix.resolve())) + + def test_own_namespace_is_refused(self): + proc = self.process(10) + self.assertEqual( + supervisor.namespace_members(self.ns, self.root, own_namespace=self.ns), []) + self.assertEqual( + [pid for pid, _proc in supervisor.namespace_members(self.ns, self.root, + 'pid:[1]')], [10]) + + def test_unreadable_environ_is_not_owned(self): + proc = self.process(10, environ=None) + self.assertFalse(supervisor.owned_by_prefix(proc, self.prefix.resolve())) + + def test_game_is_matched_through_the_prefix_socket(self): + proc = self.process(10, comm='LeagueofLegends', + environ=b'__mldr_sockpath=%s\0' % os.fsencode(self.socket)) + (proc / 'cmdline').write_bytes(b'/Applications/LeagueofLegends\0') + self.assertTrue(supervisor.game_running(self.prefix, self.root)) + other = self.root / 'other' + other.mkdir() + self.assertFalse(supervisor.game_running(other, self.root)) + + +class CleanupTests(unittest.TestCase): + def setUp(self): + self.temp = tempfile.TemporaryDirectory() + self.addCleanup(self.temp.cleanup) + self.root = Path(self.temp.name) + self.prefix = self.root / '.darling' + self.ns = 'pid:[4026532000]' + self.own = 'pid:[4026531000]' + self.output = io.StringIO() + + def process(self, pid, ppid=1): + return fake_process(self.root, pid, ppid=ppid, ns=self.ns) + + def test_cleanup_shuts_down_then_terminates_namespace_members(self): + self.process(10) + self.process(11, ppid=10) + calls = [] + killed = [] + + def run(command, **_kwargs): + calls.append(command) + + def kill(pid, signum): + killed.append((pid, signum)) + if signum == supervisor.signal.SIGTERM: + import shutil + shutil.rmtree(self.root / str(pid)) + + remaining = supervisor.cleanup( + self.prefix, '/usr/bin/darling', self.ns, proc_root=self.root, + run=run, kill=kill, sleep=lambda _seconds: None, out=self.output, + own_namespace=self.own, term_grace=0, kill_grace=0) + self.assertEqual(remaining, []) + self.assertEqual(calls, [['/usr/bin/darling', 'shutdown']]) + # Deepest process first: the child (11) before its parent (10). + self.assertEqual(killed, [(11, supervisor.signal.SIGTERM), + (10, supervisor.signal.SIGTERM)]) + self.assertIn('guest session stopped', self.output.getvalue()) + + def test_cleanup_does_nothing_without_attribution(self): + output = io.StringIO() + remaining = supervisor.cleanup( + self.prefix, '/usr/bin/darling', None, proc_root=self.root, + run=lambda *_a, **_k: self.fail('must not run darling shutdown'), + kill=lambda *_a: self.fail('must not kill'), sleep=lambda _s: None, + out=output, own_namespace=self.own, term_grace=0, kill_grace=0) + self.assertEqual(remaining, []) + self.assertIn('gone', output.getvalue()) + + def test_live_survivor_is_reported(self): + self.process(10) + output = io.StringIO() + remaining = supervisor.cleanup( + self.prefix, None, self.ns, proc_root=self.root, + run=lambda *_a, **_k: None, kill=lambda *_a: None, + sleep=lambda _s: None, out=output, own_namespace=self.own, term_grace=0, kill_grace=0) + self.assertEqual([pid for pid, _proc in remaining], [10]) + self.assertIn('still present: 10', output.getvalue()) + + +class RuntimeMarkerTests(unittest.TestCase): + def setUp(self): + self.temp = tempfile.TemporaryDirectory() + self.addCleanup(self.temp.cleanup) + self.root = Path(self.temp.name) + self.proc = self.root / 'proc' + self.proc.mkdir() + self.prefix = self.root / '.darling' + self.prefix.mkdir() + + def server(self, pid=10, argv=None, state='S'): + proc = fake_process(self.proc, pid, state=state, comm='darlingserver') + (proc / 'cmdline').write_bytes( + b'\0'.join(os.fsencode(part) for part in + (argv or ['/usr/libexec/darlingserver', str(self.prefix)])) + b'\0') + (self.prefix / '.init.pid').write_text('%d\n' % pid) + return proc + + def test_live_server_of_this_prefix_is_recognised(self): + self.server() + self.assertTrue(supervisor.runtime_alive(self.prefix, self.proc)) + + def test_missing_marker_is_not_a_runtime(self): + self.assertFalse(supervisor.runtime_alive(self.prefix, self.proc)) + + def test_reused_pid_is_not_a_runtime(self): + self.server() + proc = self.proc / '10' + (proc / 'cmdline').write_bytes(b'/usr/bin/other\0\0') + self.assertFalse(supervisor.runtime_alive(self.prefix, self.proc)) + + def test_other_prefix_is_not_ours(self): + self.server(argv=['/usr/libexec/darlingserver', '/elsewhere/.darling']) + self.assertFalse(supervisor.runtime_alive(self.prefix, self.proc)) + + def test_zombie_server_is_not_a_runtime(self): + self.server(state='Z') + self.assertFalse(supervisor.runtime_alive(self.prefix, self.proc)) + + +class SupervisorModeTests(unittest.TestCase): + """run_supervised must not end a runtime that another session started.""" + + def setUp(self): + self.temp = tempfile.TemporaryDirectory() + self.addCleanup(self.temp.cleanup) + self.root = Path(self.temp.name) + self.prefix = self.root / '.darling' + self.prefix.mkdir() + self.proc = self.root / 'proc' + self.proc.mkdir() + + def run_supervisor(self): + output = io.StringIO() + code = supervisor.run_supervised( + ['true'], self.prefix, None, proc_root=self.proc, poll=0, + run=lambda *_a, **_k: None, kill=lambda *_a: None, + sleep=lambda _s: None, out=output) + self.assertEqual(code, 0) + return output.getvalue() + + def test_own_session_is_cleaned_up(self): + self.assertIn('gone', self.run_supervisor()) + + def test_attached_session_is_left_running(self): + proc = fake_process(self.proc, 10, comm='darlingserver') + (proc / 'cmdline').write_bytes( + b'/usr/libexec/darlingserver\0' + os.fsencode(self.prefix) + b'\0') + (self.prefix / '.init.pid').write_text('10\n') + output = self.run_supervisor() + self.assertIn('another session owns this runtime', output) + self.assertNotIn('guest session stopped', output) + + +class IdleDecisionTests(unittest.TestCase): + def test_window_present_resets_the_timer(self): + self.assertEqual(supervisor.idle_decision(True, 100.0, 200.0, True, False, 60), + (True, None, False)) + + def test_window_gone_starts_and_then_fires(self): + seen, since, stop = supervisor.idle_decision(True, None, 100.0, False, False, 60) + self.assertEqual((seen, since, stop), (True, 100.0, False)) + self.assertEqual(supervisor.idle_decision(True, since, 150.0, False, False, 60), + (True, 100.0, False)) + self.assertEqual(supervisor.idle_decision(True, since, 161.0, False, False, 60), + (True, 100.0, True)) + + def test_game_resets_the_timer(self): + self.assertEqual(supervisor.idle_decision(True, 100.0, 200.0, False, True, 60), + (True, None, False)) + + def test_unknown_window_state_never_stops(self): + self.assertEqual(supervisor.idle_decision(True, 100.0, 200.0, None, False, 60), + (True, None, False)) + + def test_client_that_was_never_seen_never_stops(self): + self.assertEqual(supervisor.idle_decision(False, None, 200.0, False, False, 60), + (False, None, False)) + + def test_disabled_never_stops(self): + self.assertEqual(supervisor.idle_decision(True, 100.0, 999.0, False, False, 0), + (True, None, False)) + + +class ManagedWindowTests(unittest.TestCase): + def test_client_class_is_detected(self): + def run(command, **_kwargs): + output = ('_NET_CLIENT_LIST(WINDOW): window id # 0x1' if '_NET_CLIENT_LIST' in command + else 'WM_CLASS(STRING) = "riot client", "Riot Client"') + + class Result: + stdout = output + return Result() + + self.assertTrue(supervisor.managed_client_window(run)) + + def test_missing_property_is_unknown(self): + def run(_command, **_kwargs): + class Result: + stdout = 'not found' + return Result() + + self.assertIsNone(supervisor.managed_client_window(run)) + + +if __name__ == '__main__': + unittest.main() diff --git a/tools/run-riot-client.sh b/tools/run-riot-client.sh index bf1d067..1e50e76 100755 --- a/tools/run-riot-client.sh +++ b/tools/run-riot-client.sh @@ -158,7 +158,15 @@ fi # failed copy as "not found". Use a writable guest directory by default. # exec-guest.py closes host file descriptors above stderr before starting the guest. A shell # loop cannot: dash only accepts single-digit descriptors in redirections. -exec python3 "$runner_dir/exec-guest.py" env -u ELECTRON_RUN_AS_NODE -u DYLD_INSERT_LIBRARIES EGL_PLATFORM=x11 \ +# Keep the launcher as a supervisor: on Ctrl+C/terminal close it stops the +# guest session instead of orphaning launchd/mldr and the client tree. It also +# shuts the runtime down after the loader exits, unless another session already +# owned it. Set RCLOL_CLEAN_ON_EXIT=0 for the previous exec-only behaviour, and +# RCLOL_STOP_AFTER_CLIENT_SECONDS=N to stop once the client window has been +# gone for N seconds with no game running. +exec python3 "$runner_dir/session-supervisor.py" --prefix "$prefix" \ + --darling "${darling_bin:-darling}" -- \ + python3 "$runner_dir/exec-guest.py" env -u ELECTRON_RUN_AS_NODE -u DYLD_INSERT_LIBRARIES EGL_PLATFORM=x11 \ MAC_CHROMIUM_TMPDIR="${MAC_CHROMIUM_TMPDIR:-/private/tmp}" \ LANG="${RCLOL_GUEST_LOCALE:-en_US.UTF-8}" LC_ALL="${RCLOL_GUEST_LOCALE:-en_US.UTF-8}" \ mesa_glthread="${RCLOL_MESA_GLTHREAD-${mesa_glthread-false}}" \ diff --git a/tools/session-supervisor.py b/tools/session-supervisor.py new file mode 100755 index 0000000..5f0e140 --- /dev/null +++ b/tools/session-supervisor.py @@ -0,0 +1,442 @@ +#!/usr/bin/env python3 +"""Supervise one guest session so a launcher exit does not orphan it. + +The launcher runs this script instead of exec'ing the guest client directly. It +forwards SIGINT/SIGTERM/SIGHUP to the child, and after the child exits it asks +darlingserver to shut down and stops any process still inside the guest PID +namespace. That covers the two ways a session leaks: the Riot Client stays +resident after its window closes, and a killed loader leaves launchd/mldr and +the client tree behind. + +The guest namespace is captured while the session runs, from a process that +carries this prefix's `__mldr_sockpath`; without that evidence cleanup never +touches a process. If the namespace cannot be read, only processes that carry +the socket are stopped. Cleanup only runs when this launcher started the +runtime: a second launch that attaches to an existing server leaves it alone +when its own loader exits. + +Optional idle stop: with RCLOL_STOP_AFTER_CLIENT_SECONDS=N, the supervisor +stops the session once a managed League/Riot window has been absent for N +seconds and no game process is running. It is off by default and never stops +when the window state cannot be read. + +Set RCLOL_CLEAN_ON_EXIT=0 to keep the previous behaviour: the runtime is left +alone when the launcher exits. + +Usage (from run-riot-client.sh): + + python3 session-supervisor.py --prefix ~/.darling --darling /path/darling \ + -- python3 exec-guest.py env ... darling ... +""" +import argparse +import os +import re +import signal +import subprocess +import sys +import time +from pathlib import Path + +SOCK_VARIABLE = b'__mldr_sockpath' +SOCK_NAME = '.darlingserver.sock' +GAME_NAME = 'LeagueofLegends' +CLIENT_CLASSES = ('leagueclientux', 'riot client') +POLL_SECONDS = 2.0 +TERM_GRACE = 5.0 +KILL_GRACE = 5.0 + + +def log(message, out=sys.stdout): + """Best-effort output; a closed terminal must not abort cleanup.""" + try: + print('session-supervisor: %s' % message, file=out, flush=True) + except (OSError, ValueError): + pass + + +def process_info(proc): + """Return (state, ppid, start_ticks), or None when stat is unreadable.""" + try: + fields = (proc / 'stat').read_text().rsplit(')', 1)[1].split() + if len(fields) < 20: + return None + return fields[0], int(fields[1]), int(fields[19]) + except (OSError, ValueError, IndexError): + return None + + +def command_names(proc): + try: + raw = (proc / 'cmdline').read_bytes() + except OSError: + return [] + return [os.path.basename(part.decode(errors='replace')) + for part in raw.split(b'\0') if part] + + +def command_line(proc): + """Raw argv as text, without basenaming, for interpreter/path checks.""" + try: + raw = (proc / 'cmdline').read_bytes() + except OSError: + return [] + return [os.fsdecode(part) for part in raw.split(b'\0') if part] + + +def environment(proc): + try: + return dict( + item.split(b'=', 1) + for item in (proc / 'environ').read_bytes().split(b'\0') + if b'=' in item + ) + except OSError: + return {} + + +def socket_prefix(proc): + """The prefix named by a process's loader socket, or None.""" + raw = environment(proc).get(SOCK_VARIABLE) + if raw is None: + return None + try: + path = Path(os.fsdecode(raw)) + return path.parent.resolve() + except (OSError, RuntimeError, ValueError): + return None + + +def owned_by_prefix(proc, prefix): + if process_info(proc) is None: + return False + try: + if proc.stat().st_uid != os.getuid(): + return False + except OSError: + return False + return socket_prefix(proc) == prefix + + +def prefix_namespace(prefix, proc_root=Path('/proc')): + """The guest PID namespace, proved by a process carrying this socket.""" + for proc in proc_root.iterdir() if proc_root.exists() else (): + if not proc.name.isdigit(): + continue + if owned_by_prefix(proc, prefix): + try: + return os.readlink(proc / 'ns/pid') + except OSError: + return None + return None + + +def namespace_members(namespace, proc_root=Path('/proc'), own_namespace=None): + """Processes inside namespace, excluding this host process. + + A namespace equal to the supervisor's own is refused: a mistaken capture + must never turn session cleanup into a host-wide kill. + """ + if not namespace or namespace == own_namespace: + return [] + found = [] + for proc in proc_root.iterdir() if proc_root.exists() else (): + if not proc.name.isdigit(): + continue + try: + if os.readlink(proc / 'ns/pid') != namespace: + continue + if proc.stat().st_uid != os.getuid(): + continue + except OSError: + continue + found.append((int(proc.name), proc)) + return found + + +def prefix_only(prefix, proc_root=Path('/proc')): + """Fallback: processes that positively carry this prefix's socket.""" + found = [] + for proc in proc_root.iterdir() if proc_root.exists() else (): + if proc.name.isdigit() and owned_by_prefix(proc, prefix): + found.append((int(proc.name), proc)) + return found + + +def deepest_first(members, proc_root=Path('/proc')): + """Children before parents, so live parents can reap their own tree.""" + parents = {} + for pid, proc in members: + info = process_info(proc) + parents[pid] = info[1] if info else None + known = {pid for pid, _proc in members} + + def depth(pid, seen): + parent = parents.get(pid) + if parent not in known or parent == pid or parent in seen: + return 0 + return depth(parent, seen | {pid}) + 1 + + return sorted((pid for pid, _proc in members), + key=lambda pid: (-depth(pid, set()), pid)) + + +def wrapper(proc, name): + """Return (state, comm, ppid) for a /proc entry, or None.""" + try: + state = (proc / 'stat').read_text().rsplit(')', 1)[1].split()[0] + comm = (proc / 'comm').read_text().strip() + parent = int((proc / 'stat').read_text().rsplit(')', 1)[1].split()[1]) + return state, comm, parent + except (OSError, ValueError, IndexError): + return None + + +def stop_processes(members, kill=os.kill, sleep=time.sleep, proc_root=Path('/proc'), + out=sys.stdout, term_grace=TERM_GRACE, kill_grace=KILL_GRACE, + clock=time.monotonic): + """SIGTERM then SIGKILL a set of processes; return what is still present.""" + def alive(pid, proc): + info = process_info(proc) + return info is not None and info[0] != 'Z' + + remaining = [(pid, proc) for pid, proc in members if alive(pid, proc)] + for pid in deepest_first(remaining, proc_root): + try: + kill(pid, signal.SIGTERM) + except OSError: + pass + deadline = clock() + term_grace + while remaining and clock() < deadline: + sleep(POLL_SECONDS) + remaining = [(pid, proc) for pid, proc in remaining if alive(pid, proc)] + # Re-read once before escalating: a zero grace period must not SIGKILL a + # process that already exited on SIGTERM. + remaining = [(pid, proc) for pid, proc in remaining if alive(pid, proc)] + for pid in deepest_first(remaining, proc_root): + try: + kill(pid, signal.SIGKILL) + except OSError: + pass + deadline = clock() + kill_grace + while remaining and clock() < deadline: + sleep(POLL_SECONDS) + remaining = [(pid, proc) for pid, proc in remaining + if process_info(proc) is not None] + # Re-read once even when a grace period is zero, so the caller never gets a + # stale "still running" answer for a process that SIGKILL already removed. + return [(pid, proc) for pid, proc in remaining + if process_info(proc) is not None] + + +def cleanup(prefix, darling, namespace, proc_root=Path('/proc'), run=subprocess.run, + kill=os.kill, sleep=time.sleep, out=sys.stdout, own_namespace=None, + term_grace=TERM_GRACE, kill_grace=KILL_GRACE, clock=time.monotonic): + """Shut down the runtime, then stop anything left in the guest namespace.""" + if namespace is None: + namespace = prefix_namespace(prefix, proc_root) + own_namespace = own_namespace or os.readlink('/proc/self/ns/pid') + members = namespace_members(namespace, proc_root, own_namespace) + if members and darling: + try: + run([darling, 'shutdown'], stdout=subprocess.DEVNULL, + stderr=subprocess.DEVNULL, timeout=25) + except (OSError, subprocess.SubprocessError): + pass + deadline = clock() + term_grace + while members and clock() < deadline: + sleep(POLL_SECONDS) + members = [(pid, proc) for pid, proc in members + if process_info(proc) is not None] + if not members: + members = prefix_only(prefix, proc_root) + if not members: + log('runtime and guest processes are gone', out) + return [] + log('stopping %d leftover guest process(es)' % len(members), out) + remaining = stop_processes(members, kill=kill, sleep=sleep, + proc_root=proc_root, out=out, term_grace=term_grace, + kill_grace=kill_grace, clock=clock) + if remaining: + details = [] + for pid, proc in remaining: + info = wrapper(proc, proc_root) + details.append('%d (%s, %s)' % (pid, info[1] if info else '?', + info[0] if info else '?')) + log('still present: %s' % '; '.join(details), out) + else: + log('guest session stopped', out) + return remaining + + +def runtime_alive(prefix, proc_root=Path('/proc')): + """True when this prefix's darlingserver marker names a live server. + + Used to tell "we started the runtime" from "we attached to one that another + launcher already owns". Cleanup must never end the second kind. + """ + try: + pid = int((prefix / '.init.pid').read_text().strip()) + except (OSError, ValueError): + return False + if pid <= 0: + return False + proc = proc_root / str(pid) + info = process_info(proc) + if info is None or info[0] == 'Z': + return False + argv = command_line(proc) + if not argv or os.path.basename(argv[0]) != 'darlingserver': + return False + try: + return (proc.stat().st_uid == os.getuid() and len(argv) > 1 + and Path(argv[1]).resolve() == prefix) + except OSError: + return False + + +def managed_client_window(run=subprocess.run): + """True/False for a managed client window; None when it cannot be read.""" + try: + listing = run(['xprop', '-root', '_NET_CLIENT_LIST'], text=True, + capture_output=True, timeout=5).stdout + except (OSError, subprocess.SubprocessError): + return None + lowered = listing.lower() + if '_net_client_list' not in lowered or 'not found' in lowered: + return None + for raw in set(re.findall(r'0x[0-9a-fA-F]+', listing)): + try: + classes = run(['xprop', '-id', raw, 'WM_CLASS'], text=True, + capture_output=True, timeout=5).stdout.lower() + except (OSError, subprocess.SubprocessError): + return None + if 'not found' in classes or '=' not in classes: + return None + if any(name in classes for name in CLIENT_CLASSES): + return True + return False + + +def game_running(prefix, proc_root=Path('/proc')): + """True/False for a game of this prefix; None when /proc cannot prove it.""" + if not proc_root.exists(): + return None + for proc in proc_root.iterdir(): + if not proc.name.isdigit(): + continue + if GAME_NAME not in command_names(proc): + continue + if socket_prefix(proc) == prefix: + return True + return False + + +def idle_decision(seen_client, idle_since, now, windows, game, seconds): + """Return (seen_client, idle_since, stop_now) for one poll.""" + if seconds <= 0 or windows is None or game is None: + return seen_client, None, False + if game or windows: + return seen_client or game or windows, None, False + if not seen_client: + return False, None, False + if idle_since is None: + return True, now, False + return True, idle_since, now - idle_since >= seconds + + +def run_supervised(command, prefix, darling, stop_after=0, clean=True, + proc_root=Path('/proc'), run=subprocess.run, kill=os.kill, + sleep=time.sleep, out=sys.stdout, poll=POLL_SECONDS, + own_namespace=None): + """Run command, forward termination signals, then clean the session. + + Cleanup only runs when this launcher started the runtime. A second launch + while a session is alive attaches to that server, and must not shut it down + when its own loader exits. + """ + if not clean: + os.execvp(command[0], command) + owns_runtime = not runtime_alive(prefix, proc_root) + namespace = None + child = subprocess.Popen(command, start_new_session=True) + stop = [] + previous = {} + + def forward(signum, _frame): + stop.append(signum) + try: + os.killpg(child.pid, signal.SIGTERM) + except OSError: + pass + + for signum in (signal.SIGINT, signal.SIGTERM, signal.SIGHUP): + previous[signum] = signal.signal(signum, forward) + seen_client = False + idle_since = None + try: + while child.poll() is None: + # Learn the guest namespace while the session runs: after a crash + # the processes that still carry the socket may be the only proof. + if namespace is None: + namespace = prefix_namespace(prefix, proc_root) + if stop_after > 0: + now = time.monotonic() + seen_client, idle_since, stop_now = idle_decision( + seen_client, idle_since, now, managed_client_window(run), + game_running(prefix, proc_root), stop_after) + if stop_now: + log('client window has been gone for %ds; stopping the session' + % stop_after, out) + try: + os.killpg(child.pid, signal.SIGTERM) + except OSError: + pass + break + sleep(poll) + deadline = time.monotonic() + TERM_GRACE + while child.poll() is None and time.monotonic() < deadline: + sleep(poll) + if child.poll() is None: + try: + os.killpg(child.pid, signal.SIGKILL) + except OSError: + pass + code = child.wait() + finally: + for signum, handler in previous.items(): + signal.signal(signum, handler) + if not owns_runtime: + log('another session owns this runtime; leaving it running', out) + remaining = [] + else: + namespace = namespace or prefix_namespace(prefix, proc_root) + remaining = cleanup(prefix, darling, namespace, proc_root=proc_root, run=run, + kill=kill, sleep=sleep, out=out, + own_namespace=own_namespace) + if stop: + log('interrupted by signal %d' % stop[0], out) + return 0 if remaining else (code if code >= 0 else 128 - code) + + +def main(argv=None): + parser = argparse.ArgumentParser(description=__doc__.splitlines()[0]) + parser.add_argument('--prefix', type=Path, required=True) + parser.add_argument('--darling', help='loader used for a graceful shutdown') + parser.add_argument('--stop-after', type=int, + default=int(os.environ.get('RCLOL_STOP_AFTER_CLIENT_SECONDS', '0')), + help='stop after the client window has been gone this many seconds') + parser.add_argument('--no-clean', action='store_true', + help='leave the runtime alone after the child exits') + parser.add_argument('command', nargs=argparse.REMAINDER) + args = parser.parse_args(argv) + command = args.command[1:] if args.command[:1] == ['--'] else args.command + if not command: + parser.error('a command is required after --') + clean = not args.no_clean and os.environ.get('RCLOL_CLEAN_ON_EXIT', '1') != '0' + prefix = args.prefix.expanduser().resolve() + return run_supervised(command, prefix, args.darling, stop_after=max(0, args.stop_after), + clean=clean) + + +if __name__ == '__main__': + sys.exit(main()) From b4e4a0c59be7681e92c15a4f260d2e823e6c79dc Mon Sep 17 00:00:00 2001 From: zero Date: Sun, 25 Oct 2026 13:00:23 -0600 Subject: [PATCH 2/3] test(launcher): follow the supervisor handoff in the offline check The stub records the final darling launch after exec-guest.py. The launcher now runs session-supervisor.py first, so the stub hands the supervised command to exec-guest.py instead of stopping at the supervisor. The guest assertions are unchanged: they still inspect the launch that exec-guest.py performs. --- tools/riot-launcher-offline-check.py | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/tools/riot-launcher-offline-check.py b/tools/riot-launcher-offline-check.py index 0280d4e..d280b32 100644 --- a/tools/riot-launcher-offline-check.py +++ b/tools/riot-launcher-offline-check.py @@ -23,6 +23,15 @@ def check(): stub = f'''#!{sys.executable} import json, os, sys record = {{"command": os.path.basename(sys.argv[0]), "args": sys.argv[1:]}} +# The launcher hands the guest to the session supervisor before exec-guest.py. +# Run the supervised command without recording the supervisor itself, so the +# existing assertions still inspect the guest launch that follows. +if record["command"] == "python3" and sys.argv[1:] and os.path.basename(sys.argv[1]) == "session-supervisor.py": + remainder = sys.argv[2:] + if "--" in remainder: + command = remainder[remainder.index("--") + 1:] + if command and command[0] == "python3": + os.execv(sys.executable, [sys.executable] + command[1:]) # The launcher starts the guest through exec-guest.py; run it for real. if record["command"] == "python3" and sys.argv[1:] and os.path.basename(sys.argv[1]) == "exec-guest.py": os.execv(sys.executable, [sys.executable] + sys.argv[1:]) From 6b8cf67ca42c6db4daa38dce54f70ed9a124eca1 Mon Sep 17 00:00:00 2001 From: zero Date: Sun, 25 Oct 2026 14:25:29 -0600 Subject: [PATCH 3/3] fix(launcher): escalate after a forwarded signal and keep a failed cleanup failed - a forwarded SIGINT/SIGTERM/SIGHUP now leaves the polling loop, so the five-second TERM grace and the SIGKILL fallback actually run; a loader that ignores SIGTERM no longer blocks the supervisor forever - surviving guest processes now return a non-zero status instead of 0 - an invalid RCLOL_STOP_AFTER_CLIENT_SECONDS warns and disables the idle stop instead of aborting the launch with a traceback Adds a regression test that sends SIGTERM to a loader and a descendant that both ignore it, and asserts the process group is killed. Signed-off-by: zero --- tests/test_session_supervisor.py | 127 +++++++++++++++++++++++++++++++ tools/session-supervisor.py | 20 ++++- 2 files changed, 144 insertions(+), 3 deletions(-) diff --git a/tests/test_session_supervisor.py b/tests/test_session_supervisor.py index 476e4d0..a2f5d8f 100644 --- a/tests/test_session_supervisor.py +++ b/tests/test_session_supervisor.py @@ -3,7 +3,11 @@ import importlib.util import io import os from pathlib import Path +import signal +import sys import tempfile +import threading +import time import unittest ROOT = Path(__file__).resolve().parents[1] @@ -207,6 +211,129 @@ class SupervisorModeTests(unittest.TestCase): self.assertNotIn('guest session stopped', output) +class ExitStatusTests(unittest.TestCase): + def setUp(self): + self.temp = tempfile.TemporaryDirectory() + self.addCleanup(self.temp.cleanup) + self.root = Path(self.temp.name) + self.prefix = self.root / '.darling' + self.prefix.mkdir() + self.proc = self.root / 'proc' + self.proc.mkdir() + + def run_supervisor(self, command, remaining): + previous = supervisor.cleanup + supervisor.cleanup = lambda *_a, **_k: remaining + self.addCleanup(setattr, supervisor, 'cleanup', previous) + return supervisor.run_supervised( + command, self.prefix, None, proc_root=self.proc, poll=0, + run=lambda *_a, **_k: None, kill=lambda *_a: None, + sleep=lambda _s: None, out=io.StringIO()) + + def test_child_status_is_kept_when_cleanup_is_clean(self): + self.assertEqual(self.run_supervisor(['sh', '-c', 'exit 3'], []), 3) + + def test_surviving_processes_force_a_failure_status(self): + self.assertEqual(self.run_supervisor(['true'], [1234]), 1) + + def test_a_failed_child_stays_failed_with_survivors(self): + self.assertEqual(self.run_supervisor(['sh', '-c', 'exit 3'], [1234]), 3) + + +class SignalEscalationTests(unittest.TestCase): + """A signal must start the TERM grace period, then kill the whole group.""" + + CHILD = ( + "import os, signal, subprocess, sys, time\n" + "signal.signal(signal.SIGTERM, signal.SIG_IGN)\n" + "worker = subprocess.Popen([sys.executable, '-c', " + "'import signal, time; signal.signal(signal.SIGTERM, signal.SIG_IGN); time.sleep(3)'])\n" + "with open(sys.argv[1], 'w') as handle:\n" + " handle.write('%d %d' % (os.getpid(), worker.pid))\n" + "time.sleep(3)\n" + ) + + def setUp(self): + self.temp = tempfile.TemporaryDirectory() + self.addCleanup(self.temp.cleanup) + self.root = Path(self.temp.name) + self.prefix = self.root / '.darling' + self.prefix.mkdir() + self.proc = self.root / 'proc' + self.proc.mkdir() + self.pids = self.root / 'pids' + self.script = self.root / 'ignore-term.py' + self.script.write_text(self.CHILD) + previous = supervisor.TERM_GRACE + supervisor.TERM_GRACE = 0.2 + self.addCleanup(setattr, supervisor, 'TERM_GRACE', previous) + + def wait_dead(self, pid, timeout=10): + deadline = time.monotonic() + timeout + while time.monotonic() < deadline: + try: + state = Path('/proc/%d/stat' % pid).read_text().rsplit(')', 1)[1].split()[0] + except (OSError, IndexError): + return True + if state == 'Z': + return True + time.sleep(0.02) + return False + + def interrupt_when_ready(self): + deadline = time.monotonic() + 20 + while not self.pids.exists() and time.monotonic() < deadline: + time.sleep(0.02) + os.kill(os.getpid(), signal.SIGTERM) + + def test_sigterm_reaches_the_grace_period_and_the_group_is_killed(self): + thread = threading.Thread(target=self.interrupt_when_ready, daemon=True) + thread.start() + output = io.StringIO() + try: + code = supervisor.run_supervised( + [sys.executable, str(self.script), str(self.pids)], self.prefix, None, + proc_root=self.proc, poll=0, run=lambda *_a, **_k: None, + kill=lambda *_a: None, sleep=lambda _s: time.sleep(0.01), out=output) + finally: + thread.join(timeout=5) + parent, worker = (int(part) for part in self.pids.read_text().split()) + self.assertEqual(code, 137) + self.assertTrue(self.wait_dead(parent), 'the loader survived the escalation') + self.assertTrue(self.wait_dead(worker), 'the descendant survived the escalation') + self.assertIn('interrupted by signal 15', output.getvalue()) + + +class IdleEnvironmentTests(unittest.TestCase): + def setUp(self): + self.previous = os.environ.get('RCLOL_STOP_AFTER_CLIENT_SECONDS') + self.addCleanup(self.restore) + + def restore(self): + if self.previous is None: + os.environ.pop('RCLOL_STOP_AFTER_CLIENT_SECONDS', None) + else: + os.environ['RCLOL_STOP_AFTER_CLIENT_SECONDS'] = self.previous + + def test_absent_value_disables_the_idle_stop(self): + os.environ.pop('RCLOL_STOP_AFTER_CLIENT_SECONDS', None) + self.assertEqual(supervisor.env_seconds('RCLOL_STOP_AFTER_CLIENT_SECONDS'), 0) + + def test_valid_value_is_used(self): + os.environ['RCLOL_STOP_AFTER_CLIENT_SECONDS'] = ' 120 ' + self.assertEqual(supervisor.env_seconds('RCLOL_STOP_AFTER_CLIENT_SECONDS'), 120) + + def test_invalid_value_warns_and_disables_the_idle_stop(self): + os.environ['RCLOL_STOP_AFTER_CLIENT_SECONDS'] = '5m' + output = io.StringIO() + self.assertEqual(supervisor.env_seconds('RCLOL_STOP_AFTER_CLIENT_SECONDS', output), 0) + self.assertIn("ignoring invalid RCLOL_STOP_AFTER_CLIENT_SECONDS='5m'", output.getvalue()) + + def test_empty_value_disables_the_idle_stop(self): + os.environ['RCLOL_STOP_AFTER_CLIENT_SECONDS'] = '' + self.assertEqual(supervisor.env_seconds('RCLOL_STOP_AFTER_CLIENT_SECONDS'), 0) + + class IdleDecisionTests(unittest.TestCase): def test_window_present_resets_the_timer(self): self.assertEqual(supervisor.idle_decision(True, 100.0, 200.0, True, False, 60), diff --git a/tools/session-supervisor.py b/tools/session-supervisor.py index 5f0e140..aeb3f64 100755 --- a/tools/session-supervisor.py +++ b/tools/session-supervisor.py @@ -374,7 +374,7 @@ def run_supervised(command, prefix, darling, stop_after=0, clean=True, seen_client = False idle_since = None try: - while child.poll() is None: + while child.poll() is None and not stop: # Learn the guest namespace while the session runs: after a crash # the processes that still carry the socket may be the only proof. if namespace is None: @@ -415,7 +415,21 @@ def run_supervised(command, prefix, darling, stop_after=0, clean=True, own_namespace=own_namespace) if stop: log('interrupted by signal %d' % stop[0], out) - return 0 if remaining else (code if code >= 0 else 128 - code) + status = code if code >= 0 else 128 - code + # A surviving guest process is a failed cleanup, not a clean exit. + return (status or 1) if remaining else status + + +def env_seconds(name, out=sys.stderr): + """Seconds from an optional environment variable; 0 when unset or invalid.""" + raw = os.environ.get(name, '').strip() + if not raw: + return 0 + try: + return int(raw) + except ValueError: + log('ignoring invalid %s=%r' % (name, raw), out) + return 0 def main(argv=None): @@ -423,7 +437,7 @@ def main(argv=None): parser.add_argument('--prefix', type=Path, required=True) parser.add_argument('--darling', help='loader used for a graceful shutdown') parser.add_argument('--stop-after', type=int, - default=int(os.environ.get('RCLOL_STOP_AFTER_CLIENT_SECONDS', '0')), + default=env_seconds('RCLOL_STOP_AFTER_CLIENT_SECONDS'), help='stop after the client window has been gone this many seconds') parser.add_argument('--no-clean', action='store_true', help='leave the runtime alone after the child exits')