fix(gateway): retry launchd bootstrap after bootout on EIO for install/start
On macOS, `launchctl bootstrap` of a label still registered in the domain fails with 5: Input/output error (EIO). That is the *already loaded* case — a stale registration from an interrupted restart or a bootout that didn't settle — recoverable by booting the leftover out and bootstrapping again, and distinct from the domain being genuinely unmanageable. launchd_install and launchd_start (both bootstrap paths) treated exit 5 as 'launchd cannot manage this macOS version' and silently degraded to a detached process, losing auto-start at login and crash-restart. Centralize bootstrap in _launchctl_bootstrap(), which on EIO boots the stale label out and retries once; only if the retry also fails does the error propagate so callers apply their existing _launchctl_domain_unsupported fallback for a genuinely broken domain. launchd_restart already boots out before bootstrapping (its drained job is almost always still registered, so a plain bootstrap would hit EIO on the common path), so it keeps its explicit pre-bootout rather than routing through the bootstrap-first helper. Corrected the stale exit-5 comment that claimed it always meant an unmanageable domain. Adds TestLaunchctlBootstrapEioRetry covering clean bootstrap (no bootout), EIO -> bootout -> retry success, persistent EIO re-raise, and non-EIO re-raise without a spurious bootout.
This commit is contained in:
HiaHia
Teknium
parent
69f08c2eb5
commit
8feeb0ccb8
2 changed files with 151 additions and 18 deletions
|
|
@ -3509,10 +3509,18 @@ def _launchd_domain() -> str:
|
|||
# the target domain, so start/restart should re-bootstrap the plist and retry.
|
||||
_LAUNCHD_JOB_UNLOADED_EXIT_CODES = frozenset({3, 113, 125})
|
||||
|
||||
# When even a fresh bootstrap can't manage the domain, launchctl returns 5
|
||||
# ("Input/output error") or a persistent 125. On those hosts launchd cannot
|
||||
# supervise the gateway at all, so we degrade to a detached background process
|
||||
# (the documented `nohup hermes gateway run` workaround). See #23387.
|
||||
# launchctl returns 5 ("Input/output error") or a persistent 125 in two very
|
||||
# different situations, so exit 5 is NOT on its own proof the domain is broken:
|
||||
# 1. The target label is still *registered* in the domain (a stale load from
|
||||
# an interrupted restart / a bootout that didn't settle). This is
|
||||
# recoverable — boot the stale label out and bootstrap again. See #42914.
|
||||
# 2. The domain genuinely can't manage services (macOS 26+, neither
|
||||
# `gui/<uid>` nor `user/<uid>` supports service management). Here launchd
|
||||
# cannot supervise the gateway at all and we degrade to a detached
|
||||
# background process (the `nohup hermes gateway run` workaround). See #23387.
|
||||
# `_launchctl_bootstrap()` disambiguates by trying the bootout+retry (case 1)
|
||||
# first; only when that retry ALSO returns 5/125 do callers treat the domain as
|
||||
# unsupported (case 2) via `_launchctl_domain_unsupported`.
|
||||
_LAUNCHCTL_DOMAIN_UNSUPPORTED_CODES = frozenset({5, 125})
|
||||
|
||||
|
||||
|
|
@ -3531,6 +3539,55 @@ def _launchctl_domain_unsupported(returncode: int) -> bool:
|
|||
return returncode in _LAUNCHCTL_DOMAIN_UNSUPPORTED_CODES
|
||||
|
||||
|
||||
# `launchctl bootstrap` returns this when the target label is *already*
|
||||
# registered in the domain — a stale load left by an interrupted restart or a
|
||||
# bootout that didn't fully settle. EIO here means "already loaded", which is
|
||||
# recoverable, NOT that the domain is unmanageable; only when a bootout + retry
|
||||
# also fails is the domain genuinely unsupported.
|
||||
_LAUNCHCTL_BOOTSTRAP_EIO = 5
|
||||
|
||||
|
||||
def _launchctl_bootstrap(
|
||||
domain: str, plist_path, label: str, *, timeout: int = 30
|
||||
) -> None:
|
||||
"""Bootstrap a launchd job, recovering from a stale already-loaded label.
|
||||
|
||||
On modern macOS, ``launchctl bootstrap`` of a label that is still
|
||||
registered in ``domain`` fails with ``5: Input/output error`` (EIO). That
|
||||
is the *already loaded* case — distinct from the domain being unmanageable,
|
||||
which callers handle via :func:`_launchctl_domain_unsupported`. A leftover
|
||||
registration from an interrupted restart leaves the job
|
||||
loaded-but-not-running, so the next bootstrap hits EIO; without this retry
|
||||
we misclassify it as "launchd cannot manage this macOS version" and degrade
|
||||
to a detached process, silently losing auto-start and crash-restart.
|
||||
|
||||
Recover by booting the stale label out and bootstrapping once more. If the
|
||||
retry still fails, the ``CalledProcessError`` propagates so callers apply
|
||||
their domain-unsupported fallback for a genuinely broken domain.
|
||||
"""
|
||||
try:
|
||||
subprocess.run(
|
||||
["launchctl", "bootstrap", domain, str(plist_path)],
|
||||
check=True,
|
||||
timeout=timeout,
|
||||
)
|
||||
return
|
||||
except subprocess.CalledProcessError as exc:
|
||||
if exc.returncode != _LAUNCHCTL_BOOTSTRAP_EIO:
|
||||
raise
|
||||
# Stale registration — drop the leftover label and bootstrap once more.
|
||||
subprocess.run(
|
||||
["launchctl", "bootout", f"{domain}/{label}"],
|
||||
check=False,
|
||||
timeout=timeout,
|
||||
)
|
||||
subprocess.run(
|
||||
["launchctl", "bootstrap", domain, str(plist_path)],
|
||||
check=True,
|
||||
timeout=timeout,
|
||||
)
|
||||
|
||||
|
||||
# ── launchd unsupported marker ─────────────────────────────────────────────
|
||||
# When launchd can't manage the domain on this host (error 5/125, macOS 26+),
|
||||
# we write a persistent marker so `launchd_status()` can explain that launchd
|
||||
|
|
@ -3858,10 +3915,8 @@ def launchd_install(force: bool = False):
|
|||
plist_path.write_text(new_plist)
|
||||
|
||||
try:
|
||||
subprocess.run(
|
||||
["launchctl", "bootstrap", _launchd_domain(), str(plist_path)],
|
||||
check=True,
|
||||
timeout=30,
|
||||
_launchctl_bootstrap(
|
||||
_launchd_domain(), plist_path, get_launchd_label(), timeout=30
|
||||
)
|
||||
except subprocess.CalledProcessError as e:
|
||||
if not _launchctl_domain_unsupported(e.returncode):
|
||||
|
|
@ -3909,11 +3964,7 @@ def launchd_start():
|
|||
plist_path.parent.mkdir(parents=True, exist_ok=True)
|
||||
plist_path.write_text(new_plist, encoding="utf-8")
|
||||
try:
|
||||
subprocess.run(
|
||||
["launchctl", "bootstrap", _launchd_domain(), str(plist_path)],
|
||||
check=True,
|
||||
timeout=30,
|
||||
)
|
||||
_launchctl_bootstrap(_launchd_domain(), plist_path, label, timeout=30)
|
||||
subprocess.run(
|
||||
["launchctl", "kickstart", f"{_launchd_domain()}/{label}"],
|
||||
check=True,
|
||||
|
|
@ -3941,11 +3992,7 @@ def launchd_start():
|
|||
# Job not loaded in this domain — re-bootstrap the plist and retry.
|
||||
print("↻ launchd job was unloaded; reloading service definition")
|
||||
try:
|
||||
subprocess.run(
|
||||
["launchctl", "bootstrap", _launchd_domain(), str(plist_path)],
|
||||
check=True,
|
||||
timeout=30,
|
||||
)
|
||||
_launchctl_bootstrap(_launchd_domain(), plist_path, label, timeout=30)
|
||||
subprocess.run(
|
||||
["launchctl", "kickstart", f"{_launchd_domain()}/{label}"],
|
||||
check=True,
|
||||
|
|
@ -4094,6 +4141,11 @@ def launchd_restart():
|
|||
print("↻ launchd job was unloaded; reloading")
|
||||
plist_path = get_launchd_plist_path()
|
||||
try:
|
||||
# Restart is the one path where the job is almost always still
|
||||
# registered (we just drained it), so a plain bootstrap would hit
|
||||
# EIO on the common case. Boot the stale label out first — cheaper
|
||||
# and clearer here than routing through _launchctl_bootstrap's
|
||||
# bootstrap-first/retry-on-EIO flow. See #23387, #42914.
|
||||
subprocess.run(
|
||||
["launchctl", "bootout", target],
|
||||
check=False,
|
||||
|
|
|
|||
|
|
@ -3388,3 +3388,84 @@ class TestServiceWorkingDirIsStable:
|
|||
# The old conditional dict form must NOT appear
|
||||
assert "SuccessfulExit" not in plist
|
||||
assert "<key>KeepAlive</key>\n <dict>" not in plist
|
||||
|
||||
|
||||
class TestLaunchctlBootstrapEioRetry:
|
||||
"""`_launchctl_bootstrap` must recover from a stale already-loaded label.
|
||||
|
||||
On macOS, ``launchctl bootstrap`` of a label that is still registered in
|
||||
the domain fails with ``5: Input/output error`` (EIO). That is the *already
|
||||
loaded* case — recoverable by booting the leftover out and retrying — not a
|
||||
sign the domain is unmanageable. The regression this guards against
|
||||
misclassified a stale registration as "launchd cannot manage this macOS
|
||||
version" and needlessly degraded the gateway to a detached process.
|
||||
"""
|
||||
|
||||
PLIST = "/tmp/ai.hermes.gateway.plist"
|
||||
DOMAIN = "gui/501"
|
||||
LABEL = "ai.hermes.gateway"
|
||||
|
||||
def test_bootstrap_succeeds_first_try_without_bootout(self, monkeypatch):
|
||||
calls = []
|
||||
|
||||
def fake_run(cmd, check=True, **kwargs):
|
||||
calls.append(cmd)
|
||||
return SimpleNamespace(returncode=0, stdout="", stderr="")
|
||||
|
||||
monkeypatch.setattr(gateway_cli.subprocess, "run", fake_run)
|
||||
|
||||
gateway_cli._launchctl_bootstrap(self.DOMAIN, self.PLIST, self.LABEL)
|
||||
|
||||
assert calls == [["launchctl", "bootstrap", self.DOMAIN, self.PLIST]]
|
||||
|
||||
def test_eio_triggers_bootout_then_retry(self, monkeypatch):
|
||||
calls = []
|
||||
|
||||
def fake_run(cmd, check=True, **kwargs):
|
||||
calls.append(cmd)
|
||||
bootstrap_calls = [c for c in calls if c[1] == "bootstrap"]
|
||||
# First bootstrap hits EIO; bootout clears it; retry succeeds.
|
||||
if cmd[1] == "bootstrap" and len(bootstrap_calls) == 1:
|
||||
raise subprocess.CalledProcessError(5, cmd)
|
||||
return SimpleNamespace(returncode=0, stdout="", stderr="")
|
||||
|
||||
monkeypatch.setattr(gateway_cli.subprocess, "run", fake_run)
|
||||
|
||||
gateway_cli._launchctl_bootstrap(self.DOMAIN, self.PLIST, self.LABEL)
|
||||
|
||||
assert calls == [
|
||||
["launchctl", "bootstrap", self.DOMAIN, self.PLIST],
|
||||
["launchctl", "bootout", f"{self.DOMAIN}/{self.LABEL}"],
|
||||
["launchctl", "bootstrap", self.DOMAIN, self.PLIST],
|
||||
]
|
||||
|
||||
def test_persistent_eio_reraises_for_domain_fallback(self, monkeypatch):
|
||||
# When the retry also fails, the error must propagate so callers apply
|
||||
# their _launchctl_domain_unsupported fallback (degrade to detached).
|
||||
def fake_run(cmd, check=True, **kwargs):
|
||||
if cmd[1] == "bootstrap":
|
||||
raise subprocess.CalledProcessError(5, cmd)
|
||||
return SimpleNamespace(returncode=0, stdout="", stderr="")
|
||||
|
||||
monkeypatch.setattr(gateway_cli.subprocess, "run", fake_run)
|
||||
|
||||
with pytest.raises(subprocess.CalledProcessError) as excinfo:
|
||||
gateway_cli._launchctl_bootstrap(self.DOMAIN, self.PLIST, self.LABEL)
|
||||
assert excinfo.value.returncode == 5
|
||||
|
||||
def test_non_eio_failure_reraises_without_bootout(self, monkeypatch):
|
||||
calls = []
|
||||
|
||||
def fake_run(cmd, check=True, **kwargs):
|
||||
calls.append(cmd)
|
||||
if cmd[1] == "bootstrap":
|
||||
raise subprocess.CalledProcessError(125, cmd)
|
||||
return SimpleNamespace(returncode=0, stdout="", stderr="")
|
||||
|
||||
monkeypatch.setattr(gateway_cli.subprocess, "run", fake_run)
|
||||
|
||||
with pytest.raises(subprocess.CalledProcessError) as excinfo:
|
||||
gateway_cli._launchctl_bootstrap(self.DOMAIN, self.PLIST, self.LABEL)
|
||||
assert excinfo.value.returncode == 125
|
||||
# A non-EIO failure is not the already-loaded case: no bootout/retry.
|
||||
assert calls == [["launchctl", "bootstrap", self.DOMAIN, self.PLIST]]
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue