fix: reap leaders during group shutdown

stelloauth/rootfs/usr/local/bin/addon-supervisor

@@ -211,6 +211,7 @@ class ProcessManager:
         self._sleep = sleep
         self._profile_path = profile_path
         self._children: list[tuple[str, subprocess.Popen]] = []
+        self._reaped_children: set[int] = set()
         self._groups: list[int] = []
         self._dead_groups: set[int] = set()
         self._stopping = False
@@ -255,7 +256,19 @@ class ProcessManager:
     def _startup_stopping(self) -> bool:
         return self._stopping or self._first_exited_child() is not None
 
+    def _reap_exited_children(self) -> None:
+        for _name, process in self._children:
+            if process.pid in self._reaped_children or process.poll() is None:
+                continue
+            try:
+                process.wait(timeout=None)
+            except OSError:
+                logging.error("Child process reap failed")
+            else:
+                self._reaped_children.add(process.pid)
+
     def _live_groups(self) -> list[int]:
+        self._reap_exited_children()
         live_groups = []
         for pgid in self._groups:
             if pgid in self._dead_groups:
@@ -291,10 +304,14 @@ class ProcessManager:
 
     def _reap_all(self) -> None:
         for _name, process in self._children:
+            if process.pid in self._reaped_children:
+                continue
             try:
                 process.wait(timeout=None)
             except OSError:
                 logging.error("Child process reap failed")
+            else:
+                self._reaped_children.add(process.pid)
 
     def _shutdown(self) -> None:
         if not self._children:

tests/test_supervisor.py

@@ -382,6 +382,7 @@ class FakeProcess:
         self.returncode: int | None = None
         self.ignores_term = ignores_term
         self.wait_calls: list[float | None] = []
+        self.on_wait = None
 
     def poll(self) -> int | None:
         return self.returncode
@@ -390,6 +391,8 @@ class FakeProcess:
         self.wait_calls.append(timeout)
         if self.returncode is None:
             self.returncode = -9
+        if self.on_wait is not None:
+            self.on_wait()
         return self.returncode
 
 
@@ -400,6 +403,7 @@ def manager_harness(
     cloak_probe=None,
     stelloauth_probe=None,
     ignores_term: tuple[bool, bool] = (False, False),
+    live_descendants: tuple[bool, bool] = (False, False),
 ):
     clock = FakeClock()
     events: list[object] = []
@@ -408,6 +412,17 @@ def manager_harness(
         FakeProcess(1002, ignores_term=ignores_term[1]),
     ]
     groups = {process.pid: True for process in processes}
+    leaders_reaped = {process.pid: False for process in processes}
+    term_delivered = {process.pid: False for process in processes}
+    for index, process in enumerate(processes):
+        def on_wait(index=index, process=process) -> None:
+            leaders_reaped[process.pid] = True
+            if not live_descendants[index] or (
+                term_delivered[process.pid] and not process.ignores_term
+            ):
+                groups[process.pid] = False
+
+        process.on_wait = on_wait
     group_checks: list[int] = []
     spawned: list[FakeProcess] = []
 
@@ -420,10 +435,17 @@ def manager_harness(
     def killpg(pid: int, sent_signal: int) -> None:
         events.append(("signal", pid, sent_signal))
         process = next(item for item in processes if item.pid == pid)
-        if sent_signal == signal.SIGKILL or not process.ignores_term:
+        if sent_signal == signal.SIGKILL:
             groups[pid] = False
             if process.returncode is None:
                 process.returncode = -sent_signal
+        else:
+            term_delivered[pid] = True
+            if not process.ignores_term:
+                if process.returncode is None:
+                    process.returncode = -sent_signal
+                if leaders_reaped[pid]:
+                    groups[pid] = False
 
     def group_alive(pid: int) -> bool:
         group_checks.append(pid)
@@ -583,7 +605,9 @@ def test_child_exit_stops_sibling_and_returns_failure(
 def test_exited_leader_with_live_descendants_still_gets_group_sigterm(
     supervisor, tmp_path: Path, monkeypatch
 ) -> None:
-    harness = manager_harness(supervisor, tmp_path)
+    harness = manager_harness(
+        supervisor, tmp_path, live_descendants=(True, False)
+    )
     triggered = False
 
     def sleep(delay: float) -> None:
@@ -603,6 +627,56 @@ def test_exited_leader_with_live_descendants_still_gets_group_sigterm(
     assert [process.wait_calls for process in harness.processes] == [[None], [None]]
 
 
+def test_leader_exit_on_sigterm_without_descendants_finishes_without_waiting(
+    supervisor, tmp_path: Path, monkeypatch
+) -> None:
+    harness = manager_harness(supervisor, tmp_path)
+
+    def stelloauth_probe() -> None:
+        harness.manager._handle_signal(signal.SIGTERM, None)
+
+    harness.manager._stelloauth_probe = stelloauth_probe
+    monkeypatch.setattr(supervisor.signal, "signal", lambda *_args: None)
+
+    assert harness.manager.run() == 0
+    assert harness.clock.now == 0.0
+    assert not any(
+        event[0] == "signal" and event[2] == signal.SIGKILL
+        for event in harness.events
+    )
+    assert [process.wait_calls for process in harness.processes] == [[None], [None]]
+
+
+def test_exited_leader_with_live_descendants_still_uses_deadline_and_sigkill(
+    supervisor, tmp_path: Path, monkeypatch
+) -> None:
+    harness = manager_harness(
+        supervisor,
+        tmp_path,
+        ignores_term=(True, False),
+        live_descendants=(True, False),
+    )
+    triggered = False
+
+    def sleep(delay: float) -> None:
+        nonlocal triggered
+        harness.clock.sleep(delay)
+        if not triggered:
+            triggered = True
+            harness.processes[0].returncode = 7
+
+    harness.manager._sleep = sleep
+    monkeypatch.setattr(supervisor.signal, "signal", lambda *_args: None)
+
+    assert harness.manager.run() == 7
+    assert harness.clock.now == pytest.approx(10.25)
+    assert ("signal", 1001, signal.SIGTERM) in harness.events
+    assert ("signal", 1001, signal.SIGKILL) in harness.events
+    assert ("signal", 1002, signal.SIGTERM) in harness.events
+    assert ("signal", 1002, signal.SIGKILL) not in harness.events
+    assert [process.wait_calls for process in harness.processes] == [[None], [None]]
+
+
 def test_group_disappearance_ends_shared_wait_without_sigkill(
     supervisor, tmp_path: Path, monkeypatch
 ) -> None: