Clean up palera1n on SIGTERM and explain an unexpected Pongo

src/orchard_cli/apple/boot.py

@@ -5,6 +5,7 @@
 import shutil
 import time
 import tempfile
+import threading
 
 from orchard_cli.state import DeviceState, private_directory
 from orchard_cli.transports import pongo
@@ -47,19 +48,32 @@ def wait_mode(device, mode, *, chip, board, product, seconds=30):
         time.sleep(0.25)
 
 
+def _exit_on_sigterm(signum, frame):
+    raise SystemExit(128 + signum)
+
+
 def run_process_group(argv, *, timeout, **kwargs):
     """subprocess.run(check=True) that also stops the command's children.
 
     palera1n posix_spawns checkra1n and only waits for it. Killing palera1n
     alone leaves checkra1n running, and it exploits the next DFU device it
     sees, so give up on the whole process group instead.
     """
-    with subprocess.Popen(argv, start_new_session=True, **kwargs) as process:
-        try:
-            returncode = process.wait(timeout=timeout)
-        except BaseException:
-            os.killpg(process.pid, signal.SIGKILL)
-            raise
+    # A plain SIGTERM (pkill, logout) would end Orchard without unwinding;
+    # turn it into SystemExit while the group runs so it is cleaned up too.
+    previous = None
+    if threading.current_thread() is threading.main_thread():
+        previous = signal.signal(signal.SIGTERM, _exit_on_sigterm)
+    try:
+        with subprocess.Popen(argv, start_new_session=True, **kwargs) as process:
+            try:
+                returncode = process.wait(timeout=timeout)
+            except BaseException:
+                os.killpg(process.pid, signal.SIGKILL)
+                raise
+    finally:
+        if previous is not None:
+            signal.signal(signal.SIGTERM, previous)
     if returncode:
         raise subprocess.CalledProcessError(returncode, argv)
 

src/orchard_cli/apple/usb.py

@@ -80,7 +80,11 @@ def selected(location, ecid, *, chip, board, mode='pongo'):
     if device['location'] != location:
         raise DeviceError('Device moved from the selected USB port')
     if device['mode'] != mode:
-        raise DeviceError(f'Expected {mode} at the selected port; found {device["mode"]}')
+        hint = ''
+        if (mode, device['mode']) == ('dfu', 'pongo'):
+            hint = ('. Another palera1n/checkra1n may have exploited it; stop any running, '
+                    'force-restart the device and enter DFU again')
+        raise DeviceError(f'Expected {mode} at the selected port; found {device["mode"]}{hint}')
     if mode in ('dfu', 'pongo') and (device['ecid'], device['chip'], device['board']) != (ecid, chip, board):
         raise DeviceError('Wrong device identity at the selected port')
     return device

tests/test_runtime.py

@@ -84,6 +84,36 @@ def test_bootstrap_timeout_stops_spawned_children(self):
             os.kill(child, 9)
             self.fail('bootstrap child outlived the timeout')
 
+    def test_bootstrap_sigterm_stops_spawned_children(self):
+        pidfile = self.root / 'child.pid'
+        script = (
+            'import sys\n'
+            'from orchard_cli.apple import boot\n'
+            'boot.run_process_group(["sh", "-c", "sleep 300 & echo $! > " + sys.argv[1] + "; wait"], timeout=60)\n')
+        env = {**os.environ, 'PYTHONPATH': str(CLI / 'src') + os.pathsep + os.environ.get('PYTHONPATH', '')}
+        orchard = subprocess.Popen([sys.executable, '-c', script, str(pidfile)], env=env)
+        try:
+            for _ in range(100):
+                if pidfile.exists() and pidfile.read_text().strip():
+                    break
+                time.sleep(0.05)
+            child = int(pidfile.read_text())
+            orchard.terminate()
+            self.assertEqual(orchard.wait(timeout=10), 128 + 15)
+        finally:
+            orchard.kill()
+        for _ in range(50):
+            try:
+                state = Path(f'/proc/{child}/stat').read_text().rsplit(')', 1)[1].split()[0]
+            except (FileNotFoundError, ProcessLookupError):  # gone mid-read
+                break
+            if state == 'Z':
+                break
+            time.sleep(0.05)
+        else:
+            os.kill(child, 9)
+            self.fail('bootstrap child outlived SIGTERM to Orchard')
+
     def test_bootstrap_failure_is_reported(self):
         from orchard_cli.apple import boot
         with self.assertRaises(subprocess.CalledProcessError):