Merge pull request #64 from DavidutzDev/feat/stop-and-signal-cleanup

Stop Darling on Ctrl+C, terminal hangup and darling-lol --stop

darling-lol-launcher.sh

@@ -24,6 +24,9 @@ if (workspace / '.launcher.lock').is_symlink():
     raise SystemExit('Refusing symlink workspace lock')
 PY
 mkdir -p "$WORKSPACE"
+if [ "${1:-}" = --stop ]; then
+  exec python3 "$SHARE_DIR/scripts/nix-stop.py" "$WORKSPACE" "$DPREFIX"
+fi
 # One mutable base runtime per workspace; do not refresh scripts mid-session.
 exec 9<>"$WORKSPACE/.launcher.lock"
 flock -n 9 || { echo 'Another darling-lol workspace session is active; use its terminal.' >&2; exit 1; }
@@ -35,7 +38,7 @@ if [ -f "$DPREFIX/.init.pid" ]; then
     state=$(awk '/^State:/ {print $2}' "/proc/$server_pid/status")
     if [ "$state" != Z ]; then
       echo 'Prefix has a live server marker; namespace reconnect is unsupported.' >&2
-      echo 'Use the original darling-lol-shell terminal to run darling shutdown, then retry. No processes were stopped.' >&2
+      echo 'Run darling-lol --stop to stop it, then retry. No processes were stopped.' >&2
       exit 1
     fi
   fi
@@ -45,16 +48,36 @@ mkdir -p "$WORKSPACE/usr-local"
 cd "$WORKSPACE"
 # Map subordinate IDs before bubblewrap sets no_new_privs. UID 0 is not host root.
 fhs_command=("$UNSHARE_BIN" --user --map-auto --map-current-user --setuid=0 --setgid=0 "$FHS_BIN")
+stop_request="$WORKSPACE/.stop-request"
+rm -f "$stop_request"
+# Ctrl+C, a closed terminal or SIGTERM must reach nix-lifecycle.py, which shuts
+# Darling down in its namespace. They would kill bubblewrap first, and
+# bubblewrap's --die-with-parent would kill the lifecycle with it. So bubblewrap
+# starts with them ignored and env restores them inside. This supervisor turns
+# them into a stop request, the same one darling-lol --stop makes.
+session() {
+  (trap '' INT QUIT HUP TERM
+   exec "${fhs_command[@]}" env --default-signal=INT,QUIT,HUP,TERM bash ./scripts/nix-session.sh "$@") 0<&0 &
+  local pid=$! status
+  trap 'touch "$stop_request"' INT HUP TERM
+  while :; do
+    wait "$pid" && status=0 || status=$?
+    kill -0 "$pid" 2>/dev/null || break
+  done
+  trap - INT HUP TERM
+  rm -f "$stop_request"
+  return "$status"
+}
 # Keep this supervisor alive to hold the lock: bubblewrap may close extra FDs.
-# It never owns or signals game processes; foreground execution carries SIGINT.
+# It never owns or signals game processes.
 case "${1:-}" in
   --shell) shift; "${fhs_command[@]}" bash "$@";;
   --update) shift; "${fhs_command[@]}" ./scripts/update.sh "$@";;
-  --installer) shift; "${fhs_command[@]}" bash ./scripts/nix-session.sh --installer "$@";;
+  --installer) shift; session --installer "$@";;
   *)
     if [ $# -gt 0 ]; then
       "${fhs_command[@]}" "$@"
     else
-      "${fhs_command[@]}" bash ./scripts/nix-session.sh
+      session
     fi;;
 esac

docs/NIX.md

@@ -57,8 +57,15 @@ for prompts and session output.
 
 Keep the terminal open while clients are running and until cleanup completes.
 Default and installer sessions wait for clients to exit, then shut Darling down
-in the same namespace. A failed shutdown is retried without discarding that
-namespace.
+in the same namespace. Crash handlers do not count as clients, because they do
+not exit on their own under Darling. A failed shutdown is retried without
+discarding that namespace.
+
+To end a session while clients are still running, press Ctrl+C in its terminal,
+close the terminal, or run `darling-lol --stop` from anywhere. Each of these
+makes the session shut Darling down and exit. If the session does not exit
+within 30 seconds, `darling-lol --stop` kills the prefix's `darlingserver`
+itself. It also stops a server that outlived its session.
 
 ## Install permanently
 
@@ -198,7 +205,7 @@ nix run github:lucannez64/darling-lol#shell
 Use the launcher-provided shell rather than entering the raw FHS environment.
 If you start Darling manually in this shell, run `darling shutdown` there
 before exiting. A separately launched shell cannot reconnect to an existing
-Darling namespace.
+Darling namespace; `darling-lol --stop` can still stop it.
 
 ## Files and operational limits
 

scripts/nix-idle.py

@@ -3,6 +3,30 @@
 import os
 from pathlib import Path
 
+# Crash handlers do not exit after their clients under Darling, so a mapping by
+# one of them alone does not mean that a client still uses the prefix.
+CRASH_HANDLERS = {'RiotClientCrashHandler', 'LeagueCrashHandler', 'chrome_crashpad_handler'}
+
+
+def crash_handler(process):
+    executable = (process / 'cmdline').read_bytes().split(b'\0', 1)[0]
+    return os.path.basename(executable.decode(errors='replace')) in CRASH_HANDLERS
+
+
+def server_running(prefix):
+    """Whether the prefix marker names a live (not zombie) process."""
+    try:
+        pid = int((prefix / '.init.pid').read_text().strip())
+    except FileNotFoundError:
+        return False
+    if pid <= 0:
+        return False
+    try:
+        status = (Path('/proc') / str(pid) / 'status').read_text()
+    except (FileNotFoundError, ProcessLookupError):
+        return False
+    return not any(line.startswith('State:') and line.split()[1] == 'Z' for line in status.splitlines())
+
 
 def client_active(prefix, *, processes=None):
     """Check exact-prefix mappings; only vanished processes may be skipped."""
@@ -22,7 +46,7 @@ def client_active(prefix, *, processes=None):
             if processes is None and (process / 'uid_map').read_text() != namespace_map:
                 continue
             mappings = (process / 'maps').read_text()
-            if any(path in mappings for path in paths):
+            if any(path in mappings for path in paths) and not crash_handler(process):
                 return True
         except (FileNotFoundError, ProcessLookupError):
             # Process exited between enumeration and inspection.
@@ -35,20 +59,13 @@ def client_active(prefix, *, processes=None):
 def main():
     prefix = Path(os.environ.get('DPREFIX', str(Path.home() / '.darling'))).resolve()
     try:
-        pid = int((prefix / '.init.pid').read_text().strip())
-    except FileNotFoundError:
-        pid = 0
-    except (OSError, ValueError):
+        running = server_running(prefix)
+    except ValueError:
         raise SystemExit('Cannot validate prefix server marker; inspect it before maintenance.')
-    if pid > 0:
-        try:
-            status = (Path('/proc') / str(pid) / 'status').read_text()
-            if not any(line.startswith('State:') and line.split()[1] == 'Z' for line in status.splitlines()):
-                raise SystemExit('Prefix has a live server marker; exit Riot/League and run darling shutdown in the ORIGINAL namespace before updating. No process was stopped.')
-        except (FileNotFoundError, ProcessLookupError):
-            pass
-        except OSError as error:
-            raise SystemExit(f'Cannot validate prefix server marker: {error}') from error
+    except OSError as error:
+        raise SystemExit(f'Cannot validate prefix server marker: {error}') from error
+    if running:
+        raise SystemExit('Prefix has a live server marker; exit Riot/League and run darling-lol --stop before updating. No process was stopped.')
     try:
         active = client_active(prefix)
     except (OSError, RuntimeError) as error:

scripts/nix-lifecycle.py

@@ -28,9 +28,14 @@ def main():
     idle = importlib.util.module_from_spec(spec)
     spec.loader.exec_module(idle)
     prefix = Path(os.environ['DPREFIX']).resolve()
+    # darling-lol --stop creates this file; the launcher removes it.
+    stop_request = Path(os.environ['DARLING_WORKSPACE']) / '.stop-request'
     child = None
     interrupted = 0
 
+    def stopping():
+        return interrupted or stop_request.exists()
+
     def interrupted_by(signum, _frame):
         nonlocal interrupted
         interrupted = 128 + signum
@@ -46,16 +51,18 @@ def main():
     result = 1
     try:
         child = subprocess.Popen(sys.argv[1:])
-        result = child.wait()
-        if result < 0:
-            result = 128 - result
+        # Poll rather than wait: guest processes can outlive the command, and a
+        # signal or stop request must still reach the shutdown below.
+        while child.poll() is None and not stopping():
+            time.sleep(0.5)
     except OSError as error:
         warn(f'Cannot start client: {error}')
     finally:
         # A launcher can return while guest clients remain. Do not tear down
-        # their server, or discard the only namespace capable of shutting it down.
+        # their server, or discard the only namespace capable of shutting it down,
+        # unless the user asked to stop.
         last_error = None
-        while True:
+        while not stopping():
             try:
                 if not idle.client_active(prefix):
                     break
@@ -66,6 +73,8 @@ def main():
                          'Exit clients to allow safe shutdown.')
                     last_error = str(error)
             time.sleep(1)
+        if stopping():
+            warn('Stopping Darling.')
         while True:
             try:
                 shutdown = subprocess.run(['bash', str(scripts / 'darling-rootless.sh'), 'shutdown']).returncode
@@ -74,10 +83,25 @@ def main():
                 shutdown = 1
             if not shutdown:
                 break
+            try:
+                # Nothing is left to shut down, for example after the server crashed.
+                if not idle.server_running(prefix):
+                    break
+            except (OSError, ValueError):
+                pass
             warn(f'Darling namespace-local shutdown failed (status {shutdown}); '
                  'retaining the original namespace and retrying in 5 seconds. '
                  'Keep this terminal open.')
             time.sleep(5)
+        if child is not None:
+            # Without its server the command exits; it is our own child to kill.
+            try:
+                result = child.wait(timeout=10)
+            except subprocess.TimeoutExpired:
+                child.kill()
+                result = child.wait()
+            if result < 0:
+                result = 128 - result
         result = interrupted or result
     return result
 

scripts/nix-stop.py

@@ -0,0 +1,104 @@
+#!/usr/bin/env python3
+"""darling-lol --stop: end the workspace's session and its Darling server.
+
+The session shuts Darling down in its own namespace when it finds the stop
+request. Only if it does not finish in time, or if no session holds the lock
+but the prefix server is still alive, is that server killed from here. The
+marker PID alone is not trusted: the process must be a darlingserver of this
+user for this prefix.
+"""
+import fcntl
+import os
+from pathlib import Path
+import signal
+import sys
+import time
+
+WAIT_SECONDS = 30
+
+
+def session_running(lock):
+    try:
+        fcntl.flock(lock, fcntl.LOCK_EX | fcntl.LOCK_NB)
+    except BlockingIOError:
+        return True
+    fcntl.flock(lock, fcntl.LOCK_UN)
+    return False
+
+
+def wait_for_session(lock):
+    deadline = time.monotonic() + WAIT_SECONDS
+    while session_running(lock):
+        if time.monotonic() > deadline:
+            return False
+        time.sleep(0.5)
+    return True
+
+
+def prefix_server(prefix):
+    """PID of this prefix's darlingserver, or 0 if none is running."""
+    try:
+        pid = int((prefix / '.init.pid').read_text().strip())
+    except (FileNotFoundError, ValueError):
+        return 0
+    process = Path('/proc') / str(pid)
+    try:
+        argv = (process / 'cmdline').read_bytes().split(b'\0')
+        owner = process.stat().st_uid
+    except (FileNotFoundError, ProcessLookupError):
+        return 0
+    if (pid <= 0 or owner != os.getuid() or len(argv) < 2
+            or os.path.basename(argv[0]) != b'darlingserver'
+            or Path(os.fsdecode(argv[1])).resolve() != prefix):
+        return 0
+    return pid
+
+
+def kill_server(pid):
+    """Same as darling shutdown: kill launchd, which ends its PID namespace, then the server."""
+    children = []
+    for task in (Path('/proc') / str(pid) / 'task').glob('*/children'):
+        try:
+            children += [int(child) for child in task.read_text().split()]
+        except (FileNotFoundError, ProcessLookupError):
+            pass
+    for target in [*children, pid]:
+        try:
+            os.kill(target, signal.SIGKILL)
+        except ProcessLookupError:
+            pass
+
+
+def main():
+    workspace, prefix = Path(sys.argv[1]), Path(sys.argv[2]).resolve()
+    request = workspace / '.stop-request'
+    workspace.mkdir(parents=True, exist_ok=True)
+    with open(workspace / '.launcher.lock', 'a+') as lock:
+        if session_running(lock):
+            print('Asking the darling-lol session to shut Darling down...', flush=True)
+            request.touch()
+            if wait_for_session(lock):
+                request.unlink(missing_ok=True)
+                print('Stopped.')
+                return 0
+            print(f'The session did not stop within {WAIT_SECONDS} seconds.', flush=True)
+        server = prefix_server(prefix)
+        if server:
+            print(f'Killing the Darling server of {prefix} (PID {server}).', flush=True)
+            kill_server(server)
+        elif not session_running(lock):
+            print('No darling-lol session is running.')
+            return 0
+        if wait_for_session(lock):
+            request.unlink(missing_ok=True)
+            print('Stopped.')
+            return 0
+        # Do not leave a request that would stop the next session at once.
+        request.unlink(missing_ok=True)
+        print('Darling is stopped, but the session that started it is still running. '
+              'Close its terminal.', file=sys.stderr)
+        return 1
+
+
+if __name__ == '__main__':
+    raise SystemExit(main())

tests/test_nix_launcher.py

@@ -1,8 +1,11 @@
 """Regression tests for the Nix launcher; no root or game account required."""
+import fcntl
 import os
 from pathlib import Path
+import signal
 import subprocess
 import tempfile
+import time
 import unittest
 
 ROOT = Path(__file__).resolve().parents[1]
@@ -20,6 +23,7 @@ class LauncherTests(unittest.TestCase):
         (self.share / 'scripts/run.sh').write_text('new packaged script\n')
         import shutil
         shutil.copy(ROOT / 'scripts/nix-copy.py', self.share / 'scripts/nix-copy.py')
+        shutil.copy(ROOT / 'scripts/nix-stop.py', self.share / 'scripts/nix-stop.py')
         self.workspace = self.home / '.local/share/darling-lol'
         self.log = self.folder / 'calls'
         self.bin = self.folder / 'bin'
@@ -62,6 +66,59 @@ class LauncherTests(unittest.TestCase):
         self.assertIn('--setgid=0', calls)
         self.assertEqual(calls[-3:], [str(self.bin / 'fhs'), 'echo', 'hello world'])
 
+    def test_session_restores_signals_inside_the_namespace(self):
+        result = self.launch()
+        self.assertEqual(result.returncode, 0, result.stderr)
+        self.assertEqual(self.log.read_text().splitlines()[-5:],
+                         [str(self.bin / 'fhs'), 'env', '--default-signal=INT,QUIT,HUP,TERM',
+                          'bash', './scripts/nix-session.sh'])
+
+    def test_interrupt_becomes_a_stop_request_and_namespace_survives_it(self):
+        # Stands in for bubblewrap: it must outlive Ctrl+C and see the request.
+        (self.bin / 'unshare').write_text(
+            '#!/bin/sh\necho started > "$TEST_LOG"\n'
+            'while [ ! -e .stop-request ]; do sleep 0.1; done\n'
+            'echo stopped >> "$TEST_LOG"\n')
+        launcher = subprocess.Popen(['bash', str(self.launcher)], env=self.env, text=True,
+                                    stdout=subprocess.PIPE, stderr=subprocess.PIPE,
+                                    start_new_session=True)
+        for _ in range(100):
+            if self.log.exists():
+                break
+            time.sleep(0.1)
+        os.killpg(launcher.pid, signal.SIGINT)
+        _, stderr = launcher.communicate(timeout=10)
+        self.assertEqual(launcher.returncode, 0, stderr)
+        self.assertEqual(self.log.read_text().splitlines(), ['started', 'stopped'])
+        self.assertFalse((self.workspace / '.stop-request').exists())
+
+    def test_stop_without_session_does_not_enter_the_namespace(self):
+        result = self.launch('--stop')
+        self.assertEqual(result.returncode, 0, result.stderr)
+        self.assertIn('No darling-lol session is running', result.stdout)
+        self.assertFalse(self.log.exists())
+
+    def test_stop_asks_the_session_holding_the_lock_to_exit(self):
+        self.workspace.mkdir(parents=True)
+        lock = self.workspace / '.launcher.lock'
+        session = subprocess.Popen(['flock', str(lock), 'sh', '-c',
+                                    'while [ ! -e "$0/.stop-request" ]; do sleep 0.1; done',
+                                    str(self.workspace)])
+        self.addCleanup(session.kill)
+        with open(lock, 'a+') as handle:
+            for _ in range(100):
+                try:
+                    fcntl.flock(handle, fcntl.LOCK_EX | fcntl.LOCK_NB)
+                    fcntl.flock(handle, fcntl.LOCK_UN)
+                    time.sleep(0.1)
+                except BlockingIOError:
+                    break
+        result = self.launch('--stop')
+        self.assertEqual(result.returncode, 0, result.stderr)
+        self.assertIn('Stopped.', result.stdout)
+        self.assertEqual(session.wait(timeout=5), 0)
+        self.assertFalse(self.log.exists())
+
     def test_setup_never_makes_a_user_writable_runtime_setuid(self):
         result = self.launch('--installer')
         self.assertEqual(result.returncode, 0, result.stderr)

tests/test_nix_lifecycle.py

@@ -3,8 +3,10 @@ import importlib.util
 import os
 from pathlib import Path
 import shutil
+import signal
 import subprocess
 import tempfile
+import time
 import unittest
 from unittest.mock import MagicMock, patch
 
@@ -30,6 +32,26 @@ class IdleTests(unittest.TestCase):
         (process / 'maps').read_text.side_effect = FileNotFoundError()
         self.assertFalse(idle.client_active(Path('/fixture/prefix'), processes=[process]))
 
+    def test_crash_handlers_alone_do_not_keep_the_prefix_active(self):
+        spec = importlib.util.spec_from_file_location('nix_idle_handlers', ROOT / 'scripts/nix-idle.py')
+        assert spec is not None and spec.loader is not None
+        idle = importlib.util.module_from_spec(spec)
+        spec.loader.exec_module(idle)
+        with tempfile.TemporaryDirectory() as folder:
+            prefix = Path(folder) / 'prefix'
+            app = prefix / 'Applications/League of Legends.app'
+
+            def process(pid, executable):
+                entry = Path(folder) / 'proc' / str(pid)
+                entry.mkdir(parents=True)
+                (entry / 'maps').write_text(f'7f00-7f01 r-xp 0 0:0 1 {app}/Contents/MacOS/{executable}\n')
+                (entry / 'cmdline').write_bytes(f'/Applications/League of Legends.app/Contents/MacOS/{executable}\0--flag\0'.encode())
+                return entry
+
+            handlers = [process(1, 'LeagueCrashHandler'), process(2, 'chrome_crashpad_handler')]
+            self.assertFalse(idle.client_active(prefix, processes=handlers))
+            self.assertTrue(idle.client_active(prefix, processes=[*handlers, process(3, 'LeagueClientUx')]))
+
 
 class LifecycleTests(unittest.TestCase):
     def setUp(self):
@@ -54,9 +76,16 @@ def client_active(prefix, **kwargs):
     with open(os.environ['TEST_LOG'], 'a') as f:
         f.write('scan\\n')
     return calls < 2
+def server_running(prefix):
+    return os.environ.get('SERVER_RUNNING', '1') == '1'
 ''')
         (scripts / 'darling-rootless.sh').write_text('''#!/bin/bash
 echo "shutdown:$*" >> "$TEST_LOG"
+# Like darling shutdown, end the client command that the server ran.
+if [ -n "${CLIENT_SLEEP:-}" ]; then
+  for _ in 1 2 3 4 5 6 7 8 9 10; do [ -e "$TEST_LOG.pid" ] && break; sleep 0.2; done
+fi
+[ -e "$TEST_LOG.pid" ] && kill "$(cat "$TEST_LOG.pid")"
 if [ "${SHUTDOWN_RC:-0}" != 0 ] && [ ! -e "$TEST_LOG.retried" ]; then
   touch "$TEST_LOG.retried"
   exit "$SHUTDOWN_RC"
@@ -65,6 +94,10 @@ exit 0
 ''')
         client = scripts / 'run.sh'
         client.write_text('''#!/usr/bin/env bash
+if [ -n "${CLIENT_SLEEP:-}" ]; then
+  echo "$$" > "$TEST_LOG.pid"
+  exec sleep "$CLIENT_SLEEP"
+fi
 echo client-finished >> "$TEST_LOG"
 exit "${CLIENT_RC:-0}"
 '''.replace('#!/usr/bin/env bash', '#!' + str(shutil.which('bash'))))
@@ -89,11 +122,38 @@ exit "${CLIENT_RC:-0}"
         self.assertEqual(self.log.read_text().splitlines().count('shutdown:shutdown'), 2)
         self.assertIn('shutdown:shutdown', self.log.read_text())
 
+    def test_stop_request_shuts_down_without_waiting_for_command_or_clients(self):
+        (self.root / '.stop-request').touch()
+        result = self.run_session(CLIENT_SLEEP='60')
+        self.assertEqual(result.returncode, 128 + 15, result.stderr)
+        self.assertIn('Stopping Darling', result.stderr)
+        self.assertEqual(self.log.read_text().splitlines(), ['shutdown:shutdown'])
+
+    def test_interrupt_shuts_down_without_waiting_for_command_or_clients(self):
+        session = subprocess.Popen(['bash', str(self.root / 'scripts/nix-session.sh')],
+                                   env=dict(self.env, CLIENT_SLEEP='60'), text=True,
+                                   stderr=subprocess.PIPE)
+        pid_file = Path(str(self.log) + '.pid')
+        for _ in range(50):
+            if pid_file.exists():
+                break
+            time.sleep(0.1)
+        session.send_signal(signal.SIGINT)
+        _, stderr = session.communicate(timeout=10)
+        self.assertEqual(session.returncode, 128 + signal.SIGINT, stderr)
+        self.assertEqual(self.log.read_text().splitlines(), ['shutdown:shutdown'])
+
+    def test_failed_shutdown_is_not_retried_once_the_server_is_gone(self):
+        result = self.run_session(SHUTDOWN_RC='23', SERVER_RUNNING='0')
+        self.assertEqual(result.returncode, 0, result.stderr)
+        self.assertEqual(self.log.read_text().splitlines().count('shutdown:shutdown'), 1)
+
     def test_shutdown_retries_when_terminal_diagnostics_fail(self):
         spec = importlib.util.spec_from_file_location('lifecycle_fixture', self.root / 'scripts/nix-lifecycle.py')
         lifecycle = importlib.util.module_from_spec(spec)
         spec.loader.exec_module(lifecycle)
         child = MagicMock()
+        child.poll.return_value = 17
         child.wait.return_value = 17
         stderr = MagicMock()
         stderr.write.side_effect = OSError('terminal disconnected')