From 058479a05326235d7a339dea3478422cb8a6dc8c Mon Sep 17 00:00:00 2001 From: lucannez64 <58551020+lucannez64@users.noreply.github.com> Date: Sun, 25 Oct 2026 19:30:16 +0100 Subject: [PATCH] fix(recovery): fail closed on inspection errors and report leftovers accurately Signed-off-by: lucannez64 <58551020+lucannez64@users.noreply.github.com> --- .github/workflows/source-check.yml | 1 + docs/TROUBLESHOOTING.md | 10 +++++--- scripts/prepare.py | 6 ++--- tests/test_prepare_inspection.py | 40 ++++++++++++++++++++++++++++++ tests/test_recover_stale_client.py | 3 ++- tools/recover-stale-client.py | 13 ++++++---- 6 files changed, 60 insertions(+), 13 deletions(-) create mode 100644 tests/test_prepare_inspection.py diff --git a/.github/workflows/source-check.yml b/.github/workflows/source-check.yml index 5543ea8..9067404 100644 --- a/.github/workflows/source-check.yml +++ b/.github/workflows/source-check.yml @@ -38,6 +38,7 @@ jobs: - 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_launchd_orphan_reaping.py' + - run: python -m unittest discover -s tests -p 'test_prepare_inspection.py' - run: python -m unittest discover -s tests -p 'test_clean_stale_helpers.py' - run: python -m unittest discover -s tests -p 'test_hide_league_client_in_game.py' - run: python -m unittest discover -s tests -p 'test_client_in_game_tricks.py' diff --git a/docs/TROUBLESHOOTING.md b/docs/TROUBLESHOOTING.md index 456f5f8..50034ed 100644 --- a/docs/TROUBLESHOOTING.md +++ b/docs/TROUBLESHOOTING.md @@ -158,10 +158,12 @@ the normal launcher start the product again. It recognizes `LeagueClientUx`, is running or when a managed client window is visible, and fails closed when X11/Xwayland properties are missing or unreadable. This saves the Darling bootstrap but not the Riot Client/CEF startup itself. The stale tree is signalled -deepest-first so live parents can still reap their own children; anything that is -left over is reparented to the guest launchd, which reaps untracked zombies -within a few seconds, so a full runtime restart is not needed to clear them. If no -window appears afterward, use a full runtime shutdown/restart. +deepest-first so live parents can still reap their own children. Guest launchd +periodically reaps untracked orphaned zombie children. Recovery reports the PID, +state and parent of each process still present after SIGKILL; a surviving live +process or a zombie owned by another parent may require further investigation. +If processes remain or no window appears afterward, use a full runtime +shutdown/restart. ### Multiple client-in-game helpers diff --git a/scripts/prepare.py b/scripts/prepare.py index 65665f4..155875b 100644 --- a/scripts/prepare.py +++ b/scripts/prepare.py @@ -17,11 +17,11 @@ LFS_URL='lfs.url=https://github.com/darlinghq/darling-swift.git/info/lfs' LFS_FILTERS=['-c','filter.lfs.required=true','-c','filter.lfs.smudge=git-lfs smudge -- %f', '-c','filter.lfs.process=git-lfs filter-process','-c','filter.lfs.clean=git-lfs clean -- %f'] def submodules(checkout): - out=subprocess.run(['git','-C',str(checkout),'submodule','status','--recursive'],text=True,capture_output=True).stdout + out=subprocess.run(['git','-C',str(checkout),'submodule','status','--recursive'],text=True,capture_output=True,check=True).stdout return [checkout/line.split()[1] for line in out.splitlines() if len(line.split())>1] def pointers(repo): """Tracked files still checked out as LFS pointer text instead of their real content.""" - out=subprocess.run(['git','-C',str(repo),'lfs','ls-files'],text=True,capture_output=True).stdout + out=subprocess.run(['git','-C',str(repo),'lfs','ls-files'],text=True,capture_output=True,check=True).stdout return [f for f in (line.split(maxsplit=2) for line in out.splitlines()) if len(f)==3 and f[1]=='-'] def repair(checkout): """An interrupted fetch leaves a submodule with a wiped index or pointer text and 'submodule update' @@ -29,7 +29,7 @@ def repair(checkout): for repo in [checkout,*submodules(checkout)]: name=repo.relative_to(checkout) or '.' if repo!=checkout and subprocess.run(['git','-C',str(repo),'status','--porcelain'],text=True, - capture_output=True).stdout.strip(): + capture_output=True,check=True).stdout.strip(): print(f'Repairing the interrupted checkout of {name}',flush=True) run(['git','-C',str(repo),'checkout','-f','HEAD'],stdout=subprocess.DEVNULL) if pointers(repo): diff --git a/tests/test_prepare_inspection.py b/tests/test_prepare_inspection.py new file mode 100644 index 0000000..9608347 --- /dev/null +++ b/tests/test_prepare_inspection.py @@ -0,0 +1,40 @@ +"""Failed Git inspections must stop source preparation.""" +import ast +from pathlib import Path +import shutil +import subprocess +import tempfile +import unittest + + +@unittest.skipUnless(shutil.which('git'), 'Git required') +class PreparationInspectionTests(unittest.TestCase): + def setUp(self): + tree = ast.parse((Path(__file__).resolve().parents[1] / 'scripts/prepare.py').read_text()) + definitions = [node for node in tree.body if isinstance(node, ast.FunctionDef) + and node.name in ('submodules', 'pointers', 'repair')] + self.namespace = {'subprocess': subprocess} + exec(compile(ast.Module(body=definitions, type_ignores=[]), 'prepare.py', 'exec'), + self.namespace) + self.temporary = tempfile.TemporaryDirectory() + self.addCleanup(self.temporary.cleanup) + self.root = Path(self.temporary.name) + + def test_failed_submodule_inspection_is_not_an_empty_list(self): + with self.assertRaises(subprocess.CalledProcessError): + self.namespace['submodules'](self.root) + + def test_failed_lfs_inspection_is_not_an_empty_list(self): + with self.assertRaises(subprocess.CalledProcessError): + self.namespace['pointers'](self.root) + + def test_failed_status_inspection_stops_repair(self): + child = self.root / 'child' + child.mkdir() + self.namespace.update(submodules=lambda _root: [child], pointers=lambda _repo: []) + with self.assertRaises(subprocess.CalledProcessError): + self.namespace['repair'](self.root) + + +if __name__ == '__main__': + unittest.main() diff --git a/tests/test_recover_stale_client.py b/tests/test_recover_stale_client.py index 1d6f1af..c1526c4 100644 --- a/tests/test_recover_stale_client.py +++ b/tests/test_recover_stale_client.py @@ -149,7 +149,8 @@ class RecoverStaleClientTests(unittest.TestCase): result, killed, output = self.run_recover(on_kill=survivor) self.assertEqual(result, 0) self.assertIn('still present after SIGKILL', output) - self.assertIn('parent PID(s): 0', output) + self.assertIn('PID 12 state=S parent=0', output) + self.assertNotIn('launchd reaps', output) def test_reused_pid_is_not_reported_as_a_leftover(self): folder = self.process(12, 'mldr', [CLIENT], self.client_env()) diff --git a/tools/recover-stale-client.py b/tools/recover-stale-client.py index 6195d9f..f9e94f3 100644 --- a/tools/recover-stale-client.py +++ b/tools/recover-stale-client.py @@ -280,11 +280,14 @@ def recover(prefix, proc_root=Path('/proc'), window_check=visible_client_window, _signal_tree(prefix, identities, order, signal.SIGKILL, kill, proc_root) leftovers = wait_for_exit(prefix, identities, proc_root, sleep) if leftovers: - parents = sorted({parent_pid(proc_root / str(pid)) for pid in leftovers} - {None}) - print('recover-stale-client: %d process(es) still present after SIGKILL ' - '(parent PID(s): %s); the guest launchd reaps untracked zombies within ' - 'seconds' % (len(leftovers), ', '.join(str(p) for p in parents) or 'unknown'), - file=out, flush=True) + details = [ + 'PID %d state=%s parent=%s' % ( + pid, process_state(proc_root / str(pid)) or 'unknown', + parent_pid(proc_root / str(pid))) + for pid in leftovers + ] + print('recover-stale-client: %d process(es) still present after SIGKILL: %s' % + (len(leftovers), '; '.join(details)), file=out, flush=True) return 0