diff --git a/.github/workflows/source-check.yml b/.github/workflows/source-check.yml index df99278..9594dc3 100644 --- a/.github/workflows/source-check.yml +++ b/.github/workflows/source-check.yml @@ -32,3 +32,4 @@ jobs: - run: python -m unittest discover -s tests -p 'test_exec_guest.py' - run: python -m unittest discover -s tests -p 'test_close_league_client_in_game.py' - run: python -m unittest discover -s tests -p 'test_recover_stale_client.py' + - run: python -m unittest discover -s tests -p 'test_clean_stale_helpers.py' diff --git a/README.md b/README.md index 111ae26..2e7ebf0 100644 --- a/README.md +++ b/README.md @@ -117,6 +117,18 @@ RCLOL_RECOVER_STALE_CLIENT=1 ./scripts/run.sh It stops only a windowless Riot/League/CEF tree, preserves `darlingserver`, launchd/mldr, sockets and `.init.pid`, and then starts the product normally. It does nothing while a game or managed client window is present and fails closed when it cannot inspect X11. See [troubleshooting](docs/TROUBLESHOOTING.md#client-remains-running-without-a-window). +If repeated launches leave several `close-league-client-in-game.py` watchers, +opt in to removing the orphaned ones before the new helper starts: + +```sh +RCLOL_CLEAN_STALE_HELPERS=1 ./scripts/run.sh +``` + +It only removes helpers of this prefix whose parent is `init`/`systemd` (the +launcher that started them has exited). A helper of a live launcher, another +prefix, the game, Riot/League/CEF, `darlingserver` and the runtime are never +touched. See [troubleshooting](docs/TROUBLESHOOTING.md#multiple-client-in-game-helpers). + ## Testing Start with standalone regression checks, then play a match manually. See [testing](docs/TESTING.md). Do not attach a debugger, query client APIs, inject diagnostic hooks or automate client/game input. This project ships no client-side automation, inspection or profiling helpers. diff --git a/docs/SETUP.md b/docs/SETUP.md index e3cafa8..c8178a6 100644 --- a/docs/SETUP.md +++ b/docs/SETUP.md @@ -86,6 +86,15 @@ launchd/mldr, sockets and `.init.pid`; if X11/Xwayland cannot be inspected it does nothing. See [troubleshooting](TROUBLESHOOTING.md#client-remains-running-without-a-window). +`RCLOL_CLEAN_STALE_HELPERS=1` runs a preflight before the match helper starts. +It removes `close-league-client-in-game.py` and +`hide-league-client-in-game.py` processes of this prefix whose parent is +`init`/`systemd`, which means the launcher that started them has exited. A +helper of a live launcher, another prefix, the game, Riot/League/CEF and +`darlingserver` are preserved. It fails closed when a command line, +environment, stat entry or parent cannot be read. See +[troubleshooting](TROUBLESHOOTING.md#multiple-client-in-game-helpers). + `RCLOL_FILTER_HOST_ENV=1` opts in to removing host desktop-session variables before the guest starts: `WAYLAND_DISPLAY`, `XDG_SESSION_*`, `XDG_CURRENT_DESKTOP`, GNOME/KDE/GDM, systemd, SSH agent, terminal, toolkit (`QT_*`, `GTK_*`, `GDK_*`), diff --git a/docs/TROUBLESHOOTING.md b/docs/TROUBLESHOOTING.md index 64d4289..ee79ad9 100644 --- a/docs/TROUBLESHOOTING.md +++ b/docs/TROUBLESHOOTING.md @@ -158,6 +158,30 @@ managed client window is visible. With X11/Xwayland unavailable it fails closed. This saves the Darling bootstrap but not the Riot Client/CEF startup itself. If no window appears afterward, use a full runtime shutdown/restart. +### Multiple client-in-game helpers + +The close/hide helper is a long-lived watcher. If an earlier launcher exits +while it remains, the next launch can start another copy; list them with: + +```sh +pgrep -af 'close-league-client-in-game.py|hide-league-client-in-game.py' +``` + +To remove the orphaned copies before the next helper starts: + +```sh +RCLOL_CLEAN_STALE_HELPERS=1 ./scripts/run.sh +``` + +Only helpers of the current prefix whose parent is `init`/`systemd` are +terminated. Helpers whose launcher is still alive, other prefixes, the game, +Riot/League/CEF, `darlingserver` and the runtime are never touched. A dry run +is available from the configured tools directory: + +```sh +RCLOL_CLEAN_STALE_HELPERS=1 python3 build/runtime/tools/clean-stale-helpers.py --dry-run +``` + ### Client window stays black after a match The League client keeps its Chromium/CEF UI (`LeagueClientUx` and its helpers) diff --git a/tests/test_clean_stale_helpers.py b/tests/test_clean_stale_helpers.py new file mode 100644 index 0000000..52c0294 --- /dev/null +++ b/tests/test_clean_stale_helpers.py @@ -0,0 +1,110 @@ +"""Exercise orphan-helper cleanup with fake /proc entries and mocked signals.""" +import importlib.util +import io +import os +from pathlib import Path +import signal +import tempfile +import unittest + +ROOT = Path(__file__).resolve().parents[1] +spec = importlib.util.spec_from_file_location( + 'clean_stale_helpers', ROOT / 'tools/clean-stale-helpers.py') +clean_stale_helpers = importlib.util.module_from_spec(spec) +spec.loader.exec_module(clean_stale_helpers) + +HELPER = '/repo/tools/close-league-client-in-game.py' +HIDE_HELPER = '/repo/tools/hide-league-client-in-game.py' + + +class CleanStaleHelpersTests(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.other = self.root / 'other-prefix' + + def parent(self, pid, comm): + folder = self.proc / str(pid) + folder.mkdir(exist_ok=True) + (folder / 'comm').write_text(comm + '\n') + + def process(self, pid, script=HELPER, parent_pid=500, parent_comm='systemd', + prefix='own', argv0='/usr/bin/python3', state='S', + environment=True, arguments=('--detach',)): + folder = self.proc / str(pid) + folder.mkdir() + argv = [argv0, script, *arguments] + (folder / 'cmdline').write_bytes( + b'\0'.join(arg.encode() for arg in argv) + b'\0') + if parent_comm is not None: + self.parent(parent_pid, parent_comm) + fields = [state, str(parent_pid)] + ['0'] * 17 + ['100'] + (folder / 'stat').write_text(f'{pid} (python3) ' + ' '.join(fields) + '\n') + if environment: + prefix = self.prefix if prefix == 'own' else prefix + (folder / 'environ').write_bytes( + b'DPREFIX=' + os.fsencode(prefix) + b'\0') + else: + (folder / 'environ').write_bytes(b'UNRELATED=1\0') + return folder + + def run_clean(self, dry_run=False): + killed = [] + + def kill(pid, signum): + killed.append((pid, signum)) + + out = io.StringIO() + result = clean_stale_helpers.clean( + self.prefix, proc_root=self.proc, kill=kill, + sleep=lambda _seconds: None, out=out, dry_run=dry_run) + return result, killed, out.getvalue() + + def test_removes_only_orphaned_helpers_of_this_prefix(self): + self.process(100, HELPER) + self.process(101, HIDE_HELPER) + self.process(200, HELPER, parent_pid=600, parent_comm='darling') + self.process(201, HELPER, prefix=self.other) + self.process(202, HELPER, argv0='/usr/bin/vim') + self.process(203, HELPER, state='Z') + self.process(204, HELPER, environment=False) + self.process(205, HELPER, parent_pid=700, parent_comm=None) + result, killed, output = self.run_clean() + self.assertEqual(result, 0) + self.assertEqual(sorted(killed), sorted([ + (100, signal.SIGTERM), (100, signal.SIGKILL), + (101, signal.SIGTERM), (101, signal.SIGKILL), + ])) + self.assertIn('removing 2 orphaned helper(s): 100,101', output) + + def test_dry_run_reports_without_signalling(self): + self.process(100) + result, killed, output = self.run_clean(dry_run=True) + self.assertEqual((result, killed), (0, [])) + self.assertIn('would remove 1 orphaned helper(s): 100', output) + + def test_no_orphans_is_a_noop(self): + self.process(200, parent_pid=600, parent_comm='darling') + result, killed, output = self.run_clean() + self.assertEqual((result, killed), (0, [])) + self.assertIn('no orphaned helpers', output) + + def test_relative_helper_prefix_is_preserved(self): + self.process(100, prefix='relative-prefix') + result, killed, output = self.run_clean() + self.assertEqual((result, killed), (0, [])) + self.assertIn('no orphaned helpers', output) + + def test_launcher_runs_cleanup_before_starting_a_helper(self): + source = (ROOT / 'tools/run-riot-client.sh').read_text() + self.assertIn('RCLOL_CLEAN_STALE_HELPERS', source) + self.assertLess(source.index('clean-stale-helpers.py'), + source.index('close-league-client-in-game.py')) + + +if __name__ == '__main__': + unittest.main() diff --git a/tools/clean-stale-helpers.py b/tools/clean-stale-helpers.py new file mode 100755 index 0000000..a01d49a --- /dev/null +++ b/tools/clean-stale-helpers.py @@ -0,0 +1,183 @@ +#!/usr/bin/env python3 +"""Remove orphaned client-in-game helpers from an earlier launcher run. + +The close/hide helpers are long-lived watch processes. A launcher can end +while they remain, and a later run starts another helper, so several copies +watch the same prefix. This opt-in preflight removes only helpers that belong +to this prefix and whose parent is init/systemd: the launcher that started +them has already exited and reparented them. It never signals a helper whose +parent is still alive, the current launcher, darlingserver, the game, +Riot/League/CEF, or a process of another prefix. + +It fails closed: an unreadable command line, environment, stat entry or +parent leaves that process alone. Enable it with +RCLOL_CLEAN_STALE_HELPERS=1; the launcher runs it before starting its own +helper. +""" + +import argparse +import os +import signal +import sys +import time +from pathlib import Path + +HELPER_NAMES = ('close-league-client-in-game.py', + 'hide-league-client-in-game.py') +REAPER_NAMES = ('init', 'systemd') +TERM_GRACE = 3.0 + + +def target_prefix(): + """The prefix this launcher owns, or None when it cannot be determined.""" + value = os.environ.get('DPREFIX', str(Path.home() / '.darling')) + prefix = Path(value).expanduser() + return prefix if prefix.is_absolute() else None + + +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 parent_is_reaper(ppid, proc_root): + """True when the parent is PID 1 or a systemd/init user manager.""" + if ppid == 1: + return True + try: + comm = (proc_root / str(ppid) / 'comm').read_text().strip() + except OSError: + return False + return comm in REAPER_NAMES + + +def helper_identity(proc, prefix, proc_root): + """Return the start ticks of an orphaned helper of prefix, or None. + + The script name must be an exact argument, and the executable must be a + Python interpreter. This avoids matching an editor or a log viewer that + happens to have the helper name in its arguments. DPREFIX is inherited by + the helper from run-riot-client.sh; a missing or different value belongs + to another install and is preserved. + """ + try: + if proc.stat().st_uid != os.getuid(): + return None + raw = (proc / 'cmdline').read_bytes() + argv = [part.decode(errors='replace') for part in raw.split(b'\0') if part] + if len(argv) < 2 or not os.path.basename(argv[0]).startswith(('python', 'pypy')): + return None + if not any(os.path.basename(arg) in HELPER_NAMES for arg in argv[1:]): + return None + environment = dict( + item.split(b'=', 1) + for item in (proc / 'environ').read_bytes().split(b'\0') + if b'=' in item + ) + raw_prefix = environment.get(b'DPREFIX') + if raw_prefix is None: + return None + helper_prefix = Path(os.fsdecode(raw_prefix)).expanduser() + if not helper_prefix.is_absolute(): + return None + helper_prefix = helper_prefix.resolve() + if helper_prefix != prefix: + return None + + info = process_info(proc) + if info is None or info[0] == 'Z': + return None + if not parent_is_reaper(info[1], proc_root): + return None + return info[2] + except FileNotFoundError: + return None + except (OSError, RuntimeError, ValueError): + return None + + +def clean(prefix, proc_root=Path('/proc'), kill=os.kill, sleep=time.sleep, + out=sys.stdout, dry_run=False): + """Terminate orphaned helpers for prefix; return 0 on every normal path.""" + if prefix is None or not prefix.is_absolute(): + print('clean-stale-helpers: unusable DPREFIX: no action', file=out, flush=True) + return 1 + try: + prefix = prefix.expanduser().resolve() + except (OSError, RuntimeError): + print('clean-stale-helpers: unusable DPREFIX: no action', file=out, flush=True) + return 1 + try: + entries = list(proc_root.iterdir()) + except OSError: + print('clean-stale-helpers: unreadable /proc: no action', file=out, flush=True) + return 0 + + helpers = {} + for proc in entries: + if not proc.name.isdigit() or int(proc.name) == os.getpid(): + continue + start_ticks = helper_identity(proc, prefix, proc_root) + if start_ticks is not None: + helpers[int(proc.name)] = start_ticks + if not helpers: + print('clean-stale-helpers: no orphaned helpers; nothing to do', + file=out, flush=True) + return 0 + + pids = sorted(helpers) + action = 'would remove' if dry_run else 'removing' + print('clean-stale-helpers: %s %d orphaned helper(s): %s' + % (action, len(pids), ','.join(str(pid) for pid in pids)), + file=out, flush=True) + if dry_run: + return 0 + + for pid, start_ticks in helpers.items(): + proc = proc_root / str(pid) + if helper_identity(proc, prefix, proc_root) != start_ticks: + continue + try: + kill(pid, signal.SIGTERM) + except OSError: + continue + sleep(TERM_GRACE) + for pid, start_ticks in helpers.items(): + proc = proc_root / str(pid) + if helper_identity(proc, prefix, proc_root) != start_ticks: + continue + try: + kill(pid, signal.SIGKILL) + except OSError: + pass + return 0 + + +def main(): + parser = argparse.ArgumentParser( + description=__doc__, formatter_class=argparse.RawDescriptionHelpFormatter) + parser.add_argument('--prefix', type=Path, + help='prefix whose helpers should be checked') + parser.add_argument('--dry-run', action='store_true', + help='report what would be removed without signalling') + args = parser.parse_args() + if os.environ.get('RCLOL_CLEAN_STALE_HELPERS') != '1': + print('clean-stale-helpers: disabled; set RCLOL_CLEAN_STALE_HELPERS=1', + flush=True) + return 0 + prefix = args.prefix or target_prefix() + if prefix is None or not prefix.is_absolute(): + print('clean-stale-helpers: unusable DPREFIX: no action', + file=sys.stderr, flush=True) + return 1 + return clean(prefix, dry_run=args.dry_run) + + +if __name__ == '__main__': + sys.exit(main()) diff --git a/tools/run-riot-client.sh b/tools/run-riot-client.sh index 085434e..34e7e06 100755 --- a/tools/run-riot-client.sh +++ b/tools/run-riot-client.sh @@ -18,6 +18,13 @@ fi if [ "${RCLOL_RECOVER_HIDDEN_CLIENT:-0}" = 1 ]; then python3 "$runner_dir/recover-hidden-riot-client.py" fi +# Opt-in cleanup of helpers left by an earlier launcher. This runs before the +# current helper is started. It removes only helpers of this prefix whose +# parent is init/systemd, so a helper of a live launcher is not touched. +if [ "${RCLOL_CLEAN_STALE_HELPERS:-0}" = 1 ]; then + python3 "$runner_dir/clean-stale-helpers.py" --prefix "$prefix_dir" || + echo 'clean-stale-helpers: cleanup failed; continuing' >&2 +fi # Opt-in: close the League client UI while a match runs. It keeps the CPU for the game # and leaves the client the backend starts when the match ends as the only CEF instance; # a second instance on the same profile never paints its window, which is the black