Reap network clients that fail before reporting readiness

test/hil/net_test.py

diff --git a/test/hil/net_test.py b/test/hil/net_test.py
index 3b0562a..93c6765 100644
--- a/test/hil/net_test.py
+++ b/test/hil/net_test.py
@@ -20,7 +20,7 @@
 import select
 import subprocess
 
-from helper import hil_util
+from helper import hil_health, hil_util
 
 DEVICE_IP = '192.168.7.1'
 # Default assets in the pinned lwIP dependency's src/apps/http/fsdata.c. Verify
@@ -70,17 +70,30 @@
     return (['sudo', '-n'] if os.geteuid() != 0 else []) + argv
 
 
-def stop_client(proc, pid):
+def stop_client(proc, pid, grace=5):
     # The unshare child runs as us, even when its sudo wrapper belongs to root.
     # Terminate that child directly so sudo can reap it and return normally.
     for sig in (signal.SIGTERM, signal.SIGKILL):
-        if pid is not None:
+        if pid is None:
+            # Startup can fail before Python reports its PID. Walk only this
+            # Popen child's descendants, leaves first, then signal the wrapper.
+            # Never killpg: these processes share the HIL caller's group.
+            children = hil_health.child_procs([proc.pid]).get(proc.pid, [])
+            for child, _pgid in reversed(children):
+                try:
+                    os.kill(child, sig)
+                except (ProcessLookupError, PermissionError):
+                    # sudo relays SIGTERM to a child still running as root before
+                    # unshare drops privileges; the caller can signal sudo itself.
+                    pass
+            proc.send_signal(sig)
+        else:
             try:
                 os.kill(pid, sig)
             except ProcessLookupError:
                 pass
         try:
-            proc.communicate(timeout=5)
+            proc.communicate(timeout=grace)
             return
         except subprocess.TimeoutExpired:
             pass

test/hil/test/test_hil_net.py

diff --git a/test/hil/test/test_hil_net.py b/test/hil/test/test_hil_net.py
index 87c1e38..e2a82da 100644
--- a/test/hil/test/test_hil_net.py
+++ b/test/hil/test/test_hil_net.py
@@ -59,7 +59,7 @@
 class NamespaceLifetime(unittest.TestCase):
     def setUp(self):
         self.calls = []
-        self.proc = Mock(returncode=0)
+        self.proc = Mock(returncode=0, pid=1234)
         self.proc.stdout = io.StringIO('4321\n')
         self.proc.poll.return_value = 0
         self.proc.communicate.return_value = ('HTTP verified\n', '')
@@ -126,6 +126,65 @@
         with self.assertRaisesRegex(RuntimeError, 'corrupt data'):
             net.check_device('ABC123')
 
+    def test_readiness_timeout_cleans_up_without_reported_pid(self):
+        net.select.select.return_value = ([], [], [])
+        self.proc.poll.return_value = None
+        with patch.object(net.hil_health, 'child_procs', return_value={1234: [(4321, 1)]}), \
+                patch.object(net.os, 'kill') as kill:
+            with self.assertRaisesRegex(RuntimeError, 'did not become ready'):
+                net.check_device('ABC123')
+        net.command.assert_not_called()
+        kill.assert_called_once_with(4321, net.signal.SIGTERM)
+        self.proc.send_signal.assert_called_once_with(net.signal.SIGTERM)
+        self.proc.communicate.assert_called_once_with(timeout=5)
+
+    def test_root_child_is_left_for_sudo_to_signal(self):
+        with patch.object(net.hil_health, 'child_procs', return_value={1234: [(4321, 1)]}), \
+                patch.object(net.os, 'kill', side_effect=PermissionError()):
+            net.stop_client(self.proc, None)
+        self.proc.send_signal.assert_called_once_with(net.signal.SIGTERM)
+        self.proc.communicate.assert_called_once_with(timeout=5)
+
+
+@unittest.skipUnless(os.name == 'posix', 'Linux process-tree cleanup')
+class UnreportedClient(unittest.TestCase):
+    def test_live_wrapper_and_descendant_without_pid_are_reaped(self):
+        # Both ignore TERM so this also exercises the KILL pass. They inherit our
+        # process group; cleanup must not signal the test runner or its siblings.
+        code = '''import os, signal, subprocess, sys, time
+signal.signal(signal.SIGTERM, signal.SIG_IGN)
+child = subprocess.Popen([sys.executable, '-c',
+    'import signal,time; signal.signal(signal.SIGTERM,signal.SIG_IGN); print("ready",flush=True); time.sleep(60)'],
+    stdout=subprocess.PIPE, text=True)
+child.stdout.readline()
+print(child.pid, flush=True)
+time.sleep(60)
+'''
+        proc = net.subprocess.Popen([sys.executable, '-c', code], stdout=net.subprocess.PIPE,
+                                    stderr=net.subprocess.PIPE, text=True)
+        child = None
+        try:
+            self.assertTrue(net.select.select([proc.stdout], [], [], 5)[0])
+            # The test knows the descendant PID; stop_client deliberately does not.
+            child = int(proc.stdout.readline())
+            with patch.object(net.os, 'killpg') as killpg:
+                net.stop_client(proc, None, grace=0.1)
+            killpg.assert_not_called()
+            self.assertIsNotNone(proc.poll())
+            stat = Path('/proc') / str(child) / 'stat'
+            if stat.exists():
+                self.assertEqual(stat.read_text().rsplit(')', 1)[1].split()[0], 'Z')
+        finally:
+            if proc.poll() is None:
+                proc.kill()
+                proc.wait(timeout=5)
+            if child is not None:
+                try:
+                    os.kill(child, net.signal.SIGKILL)
+                except ProcessLookupError:
+                    pass
+            net.hil_util._close_pipes(proc)
+
 
 class HttpVerification(unittest.TestCase):
     def setUp(self):