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.
This commit is contained in:
parent
2a2e6ee2a4
commit
fe9bdd17e3
|
|
@ -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
|
||||
|
||||
|
|
|
|||
|
|
@ -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():
|
||||
|
|
|
|||
Loading…
Reference in New Issue