From fe9bdd17e305b54b826977013f70629af59f8a77 Mon Sep 17 00:00:00 2001 From: Kshitij Kothari <82637225+kshitijk4poor@users.noreply.github.com> Date: Sun, 2 Aug 2026 23:14:16 +0530 Subject: [PATCH] fix: exclude killed PID from orphan sweep, fix test, add regression test - Pass extra_exclude={pid} to _reap_unsupervised_gateway_orphans so the killed PID isn't double-killed during the sweep (#75936). - Add extra_exclude param to _reap_unsupervised_gateway_orphans signature. - Replace bare except:pass with logger.debug for diagnosability. - Fix existing test (mock _reap_unsupervised_gateway_orphans so it doesn't scan real processes and trigger conftest live-system guard). - Add regression test asserting the killed PID is excluded from the sweep. --- hermes_cli/gateway.py | 18 ++++++++++++----- tests/hermes_cli/test_gateway.py | 33 +++++++++++++++++++++++++++++++- 2 files changed, 45 insertions(+), 6 deletions(-) diff --git a/hermes_cli/gateway.py b/hermes_cli/gateway.py index aea24118a8430..681ce09c9ef14 100644 --- a/hermes_cli/gateway.py +++ b/hermes_cli/gateway.py @@ -1504,7 +1504,7 @@ def kill_gateway_processes( return killed -def _reap_unsupervised_gateway_orphans() -> bool: +def _reap_unsupervised_gateway_orphans(extra_exclude: set | None = None) -> bool: """Kill no-supervisor gateway orphans the pidfile/runtime record can't see. On WSL/no-systemd hosts the manual restart fallback runs the gateway @@ -1517,6 +1517,10 @@ def _reap_unsupervised_gateway_orphans() -> bool: running gateway — gating on ``supports_systemd_services()`` keeps the orphan-aware scan from killing live management processes there. + Args: + extra_exclude: Additional PIDs to skip (e.g. a PID already killed by + the caller so the sweep doesn't send a redundant SIGTERM/SIGKILL). + Returns True if at least one orphan was reaped. """ try: @@ -1528,6 +1532,8 @@ def _reap_unsupervised_gateway_orphans() -> bool: from gateway.status import _pid_exists, write_planned_stop_marker own = {os.getpid()} + if extra_exclude: + own |= extra_exclude try: # find_gateway_pids() includes no-supervisor `gateway restart` runtimes # for the current profile when no systemd supervisor is present. @@ -1628,11 +1634,13 @@ def stop_profile_gateway() -> bool: remove_pid_file() # Also reap any orphans from prior restarts whose PIDs were overwritten - # in the pid file before they exited (#75936). + # in the pid file before they exited (#75936). Exclude the PID we just + # killed so the sweep doesn't double-kill a process that's still tearing + # down — _reap_unsupervised_gateway_orphans already excludes our own PID. try: - _reap_unsupervised_gateway_orphans() - except Exception: - pass + _reap_unsupervised_gateway_orphans(extra_exclude={pid} if pid else None) + except Exception as exc: + logger.debug("orphan reap after stop_profile_gateway failed: %s", exc) return True diff --git a/tests/hermes_cli/test_gateway.py b/tests/hermes_cli/test_gateway.py index f6ffd3d51ffa0..3d66de14b8966 100644 --- a/tests/hermes_cli/test_gateway.py +++ b/tests/hermes_cli/test_gateway.py @@ -369,7 +369,7 @@ class TestWaitForGatewayExit: class TestStopProfileGateway: def test_stop_profile_gateway_keeps_pid_file_when_process_still_running(self, monkeypatch): - calls = {"kill": 0, "alive_probes": 0, "remove": 0} + calls = {"kill": 0, "alive_probes": 0, "remove": 0, "reap_calls": 0} monkeypatch.setattr("gateway.status.get_running_pid", lambda: 12345) # Post-#21561: the stop loop sends one SIGTERM via ``os.kill`` then @@ -389,11 +389,42 @@ class TestStopProfileGateway: "gateway.status.remove_pid_file", lambda: calls.__setitem__("remove", calls["remove"] + 1), ) + # Mock the orphan reap so it doesn't scan for real gateway processes + # (#75936 — stop_profile_gateway now calls _reap_unsupervised_gateway_orphans + # after killing the pid-file PID). + monkeypatch.setattr( + gateway, + "_reap_unsupervised_gateway_orphans", + lambda extra_exclude=None: calls.__setitem__("reap_calls", calls["reap_calls"] + 1) or False, + ) assert gateway.stop_profile_gateway() is True assert calls["kill"] == 1 # one SIGTERM assert calls["alive_probes"] == 20 # 20 liveness polls over the 2s window assert calls["remove"] == 0 + assert calls["reap_calls"] == 1 # orphan sweep ran after kill + + def test_stop_profile_gateway_excludes_killed_pid_from_orphan_reap(self, monkeypatch): + """The PID we killed must be excluded from the orphan sweep (#75936).""" + killed_pid = 99999 + reap_extra_excludes = [] + + monkeypatch.setattr("gateway.status.get_running_pid", lambda: killed_pid) + monkeypatch.setattr(gateway.os, "kill", lambda pid, sig: None) + monkeypatch.setattr("gateway.status._pid_exists", lambda pid: False) + monkeypatch.setattr("time.sleep", lambda _: None) + monkeypatch.setattr("gateway.status.remove_pid_file", lambda: None) + + def fake_reap(extra_exclude=None): + if extra_exclude: + reap_extra_excludes.append(extra_exclude) + return False + + monkeypatch.setattr(gateway, "_reap_unsupervised_gateway_orphans", fake_reap) + + assert gateway.stop_profile_gateway() is True + assert len(reap_extra_excludes) == 1 + assert killed_pid in reap_extra_excludes[0] def test_module_has_logger():