From 31aeeb5ed32f18a7933feb96d48d06ee0d588ce7 Mon Sep 17 00:00:00 2001 From: Dennis Juhler Aagaard Date: Thu, 24 Sep 2026 17:40:13 +0200 Subject: [PATCH] fix: reap leaders during group shutdown --- .../rootfs/usr/local/bin/addon-supervisor | 17 ++++ tests/test_supervisor.py | 78 ++++++++++++++++++- 2 files changed, 93 insertions(+), 2 deletions(-) diff --git a/stelloauth/rootfs/usr/local/bin/addon-supervisor b/stelloauth/rootfs/usr/local/bin/addon-supervisor index 4ce318f..5554ab1 100755 --- a/stelloauth/rootfs/usr/local/bin/addon-supervisor +++ b/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: diff --git a/tests/test_supervisor.py b/tests/test_supervisor.py index 3c51fad..bec1a32 100644 --- a/tests/test_supervisor.py +++ b/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: