diff --git a/bot_bottle/sidecar_init.py b/bot_bottle/sidecar_init.py index 835c193..07bb92b 100644 --- a/bot_bottle/sidecar_init.py +++ b/bot_bottle/sidecar_init.py @@ -167,8 +167,8 @@ class _Supervisor: def start_all(self) -> None: for spec in self.specs: - _log(f"starting {spec.name}") self.procs.append((spec, _spawn(spec))) + _log(f"starting {spec.name}") def request_shutdown(self, reason: str) -> None: if self.shutdown_at is not None: @@ -353,8 +353,6 @@ def main(argv: Sequence[str] | None = None) -> int: return 0 sup = _Supervisor(specs) - sup.start_all() - signal.signal(signal.SIGTERM, lambda *_: sup.request_shutdown("SIGTERM")) # type: ignore signal.signal(signal.SIGINT, lambda *_: sup.request_shutdown("SIGINT")) # type: ignore # SIGHUP reload path: egress_apply.py runs `docker kill @@ -362,6 +360,7 @@ def main(argv: Sequence[str] | None = None) -> int: # delivers SIGHUP to PID 1 (this supervisor); forward it to # mitmdump so it reloads its addon. signal.signal(signal.SIGHUP, lambda *_: sup.forward_signal(signal.SIGHUP, "egress")) # type: ignore + sup.start_all() while not sup.tick(): time.sleep(_POLL_INTERVAL) diff --git a/tests/unit/test_sidecar_init.py b/tests/unit/test_sidecar_init.py index b47f913..46472f4 100644 --- a/tests/unit/test_sidecar_init.py +++ b/tests/unit/test_sidecar_init.py @@ -12,6 +12,7 @@ import os import signal import subprocess import sys +import threading import time import unittest import warnings @@ -476,6 +477,7 @@ class TestMainEndToEnd(unittest.TestCase): def _run(self, daemons_csv: str, send_signal: int | None, wait_before_signal: float = 0.4, + wait_for_output: tuple[str, ...] = (), overall_timeout: float = 6.0) -> tuple[int, str]: """Spawn sidecar_init.main() in a child process with the DAEMONS list patched to harmless `sleep 30` commands. @@ -496,19 +498,53 @@ class TestMainEndToEnd(unittest.TestCase): stdout=subprocess.PIPE, stderr=subprocess.STDOUT, env=env, ) + collected: list[bytes] = [] + ready = threading.Event() + need: set[str] = set(wait_for_output) + + def _drain() -> None: + assert proc.stdout is not None + for raw in iter(proc.stdout.readline, b""): + collected.append(raw) + if need: + text = raw.decode("utf-8", errors="replace") + need.difference_update(p for p in list(need) if p in text) + if not need: + ready.set() + ready.set() # also fires on EOF so the main thread isn't stuck + + reader = threading.Thread(target=_drain, daemon=True) + reader.start() + try: if send_signal is not None: - time.sleep(wait_before_signal) + if wait_for_output: + # Wait until all expected lines have appeared, then + # signal. This eliminates the fixed-sleep race where + # SIGTERM arrives before the subprocess installs its + # handler on a slow CI runner. + if not ready.wait(timeout=overall_timeout): + proc.kill() + reader.join(timeout=2.0) + self.fail("timed out waiting for expected output lines") + else: + time.sleep(wait_before_signal) proc.send_signal(send_signal) - out_b, _ = proc.communicate(timeout=overall_timeout) + proc.wait(timeout=overall_timeout) except subprocess.TimeoutExpired: proc.kill() - out_b, _ = proc.communicate() + proc.wait() + reader.join(timeout=2.0) self.fail("sidecar_init main() did not exit before timeout") - return proc.returncode, out_b.decode("utf-8", errors="replace") + + reader.join(timeout=2.0) + return proc.returncode, b"".join(collected).decode("utf-8", errors="replace") def test_sigterm_clean_shutdown(self): - rc, out = self._run("alpha,beta", signal.SIGTERM) + rc, out = self._run( + "alpha,beta", signal.SIGTERM, + wait_for_output=("starting alpha", "starting beta"), + ) self.assertIn("starting alpha", out) self.assertIn("starting beta", out) self.assertIn("forwarding SIGTERM", out)