fix: preserve ambiguous guest processes during prefix cleanup (#111)

* fix: preserve ambiguous guest processes during prefix cleanup

* fix: warn only for matching runtime cleanup candidates

---------

Co-authored-by: lucannez64 <[email protected]>

tools/ensure-darling-runtime.py

@@ -2,7 +2,7 @@
 """Clear a dead Darling server's stale PID marker, and stale shared buffers once no server runs."""
 import os
 from pathlib import Path
-import signal
+import sys
 import time
 def recover_marker(prefix):
     marker = prefix / '.init.pid'
@@ -38,39 +38,46 @@ def any_server_running(proc_root=Path('/proc')):
             pass
     return False
 
-def stop_orphaned_launchd(proc_root=Path('/proc')):
-    """A guest launchd that outlived its darlingserver cannot be shut down with `darling shutdown`
-    and makes the base install (tools/shutdown-user.sh) refuse to continue."""
+def _matching_pids(kind, proc_root):
+    """Find same-user candidates by guest executable; this does not establish ownership."""
     uid = os.getuid()
     for proc in proc_root.iterdir():
         if not proc.name.isdigit():
             continue
         try:
-            if proc.stat().st_uid != uid or (proc / 'cmdline').read_bytes().rstrip(b'\0') != b'/sbin/launchd':
+            if proc.stat().st_uid != uid:
                 continue
-            os.kill(int(proc.name), signal.SIGKILL)
-            print(f'Stopped orphaned Darling launchd PID {proc.name}', flush=True)
+            argv = (proc / 'cmdline').read_bytes()
+            if kind == 'launchd':
+                matches = argv.rstrip(b'\0') == b'/sbin/launchd'
+            else:
+                executable = argv.split(b'\0', 1)[0].decode(errors='replace')
+                matches = (proc / 'comm').read_text().startswith('chrome_crashpad') and any(
+                    bundle in executable for bundle in (
+                        '/Riot Client.app/', '/League of Legends.app/',
+                        '/League of Legends (TM) Client.app/'))
+            if matches:
+                yield proc.name
         except (OSError, ValueError):
-            pass
+            continue
+
+
+def stop_orphaned_launchd(proc_root=Path('/proc')):
+    """Warn about launchd candidates without assuming prefix ownership or signalling them."""
+    pids = list(_matching_pids('launchd', proc_root))
+    if pids:
+        print(f'Preserving launchd candidate PIDs {", ".join(pids)}: prefix ownership '
+              'cannot be proved. If setup is blocked, shut down the original Darling '
+              'session or inspect the leftover processes manually.', file=sys.stderr, flush=True)
+
 
 def stop_riot_crashpad(proc_root=Path('/proc')):
-    """A Riot crash handler outliving its server holds the old session's ports; spare every other app's."""
-    uid = os.getuid()
-    for proc in proc_root.iterdir():
-        if not proc.name.isdigit():
-            continue
-        try:
-            if not (proc / 'comm').read_text().startswith('chrome_crashpad') or proc.stat().st_uid != uid:
-                continue
-            argv = (proc / 'cmdline').read_bytes().split(b'\0')
-            executable = argv[0].decode(errors='replace')
-            # Match the executable's bundle, not arbitrary arguments (a browser may mention Riot).
-            if any(bundle in executable for bundle in (
-                '/Riot Client.app/', '/League of Legends.app/', '/League of Legends (TM) Client.app/'
-            )):
-                os.kill(int(proc.name), signal.SIGTERM)
-        except (OSError, ValueError):
-            pass
+    """Warn about Riot crashpad candidates without assuming prefix ownership or signalling them."""
+    pids = list(_matching_pids('crashpad', proc_root))
+    if pids:
+        print(f'Preserving Riot crashpad candidate PIDs {", ".join(pids)}: prefix ownership '
+              'cannot be proved. Use the original session to shut down leftover helpers.',
+              file=sys.stderr, flush=True)
 
 def remove_owned(paths):
     uid = os.getuid()

tools/riot-runtime-cleanup-offline-check.py

@@ -25,10 +25,9 @@ with tempfile.TemporaryDirectory(prefix='darling-cleanup-check-') as folder:
     process(104, 'unrelated', ['/usr/bin/unrelated', '/sbin/launchd'])
     with patch.object(cleanup.os, 'kill') as kill:
         cleanup.stop_riot_crashpad(root)
-        assert kill.call_args_list == [((102, cleanup.signal.SIGTERM),)]
-        kill.reset_mock()
+        kill.assert_not_called()
         cleanup.stop_orphaned_launchd(root)
-        assert kill.call_args_list == [((103, cleanup.signal.SIGKILL),)]
+        kill.assert_not_called()
     assert not cleanup.any_server_running(root)
     process(105, 'darlingserver', ['darlingserver', '--prefix=another-prefix'])
     assert cleanup.any_server_running(root)
@@ -58,4 +57,4 @@ with tempfile.TemporaryDirectory(prefix='darling-cleanup-check-') as folder:
         crashpad.assert_not_called()
         orphan.assert_not_called()
         remove.assert_not_called()
-print('RUNTIME_CLEANUP_OFFLINE pass=1 browser_preserved=1 other_prefix_preserved=1 foreign_owner_preserved=1 hidden_process_preserved=1 orphan_recovered=1')
+print('RUNTIME_CLEANUP_OFFLINE pass=1 browser_preserved=1 other_prefix_preserved=1 foreign_owner_preserved=1 hidden_process_preserved=1 orphan_preserved=1')