mirror of
https://github.com/NousResearch/hermes-agent.git
synced 2026-07-25 17:18:11 +00:00
fix(macos): use launchctl submit instead of start_new_session for plist reload helper (#69098)
The deferred launchd reload helper used start_new_session=True to detach from the gateway's process group. However, setsid(2) alone does NOT move the child outside the launchd job's process coalition — when launchctl bootout fires on the gateway label, launchd terminates ALL processes in that coalition, including the setsid-detached helper, leaving the service permanently unloaded. Fix by spawning the helper via launchctl submit, which creates a transient launchd one-shot job that is wholly independent of the gateway's coalition. This ensures the helper survives bootout and can complete the bootstrap+verify cycle. Also writes a durable pre-bootout marker to the reload log so the distinction between 'helper never started' and 'helper ran but bootout/bootstrap failed' can be diagnosed. Fixes #69098
This commit is contained in:
parent
b16e2be88f
commit
2a32fe8914
2 changed files with 42 additions and 7 deletions
|
|
@ -4124,6 +4124,11 @@ def refresh_launchd_plist_if_needed() -> bool:
|
|||
reload_log_path.parent.mkdir(parents=True, exist_ok=True)
|
||||
except OSError:
|
||||
pass
|
||||
|
||||
# Write a durable pre-bootout marker so we can distinguish "helper
|
||||
# never started" from "helper ran but bootout/bootstrap failed".
|
||||
_append_launchd_reload_log(f"Launchd reload helper started for {target}")
|
||||
|
||||
# Retry until launchctl LISTS the label (not merely a zero bootstrap
|
||||
# exit) or the drain window elapses. The failure happens while the old
|
||||
# gateway is still draining (default agent.restart_drain_timeout=180s),
|
||||
|
|
@ -4146,18 +4151,39 @@ def refresh_launchd_plist_if_needed() -> bool:
|
|||
f"fi"
|
||||
)
|
||||
try:
|
||||
# Spawn the reload helper via `launchctl submit` (a transient
|
||||
# launchd one-shot job) instead of `start_new_session=True`.
|
||||
# `start_new_session=True` only calls setsid(2), which creates a
|
||||
# new POSIX session but does NOT move the child outside the
|
||||
# launchd job's process coalition. When `launchctl bootout` fires
|
||||
# on the gateway label, launchd terminates ALL processes in that
|
||||
# coalition — including a setsid-detached child (#69098).
|
||||
#
|
||||
# `launchctl submit` creates a wholly independent transient launchd
|
||||
# job that launchd manages separately from the gateway, so bootout
|
||||
# of the gateway job cannot reach the helper.
|
||||
submit_label = f"{label}.reload.{os.getpid()}.{int(time.time())}"
|
||||
subprocess.Popen(
|
||||
["/bin/bash", "-c", reload_script],
|
||||
start_new_session=True,
|
||||
[
|
||||
"launchctl", "submit",
|
||||
"-l", submit_label,
|
||||
"-o", str(reload_log_path),
|
||||
"-e", str(reload_log_path),
|
||||
"--",
|
||||
"/bin/bash", "-c", reload_script,
|
||||
],
|
||||
stdout=subprocess.DEVNULL,
|
||||
stderr=subprocess.DEVNULL,
|
||||
)
|
||||
except Exception as e:
|
||||
logger.warning("Deferred launchd reload could not be spawned: %s", e)
|
||||
_append_launchd_reload_log(
|
||||
f"FAILED to spawn launchd reload helper for {target}: {e}"
|
||||
)
|
||||
return False
|
||||
print(
|
||||
"↻ Updated gateway launchd service definition; reload deferred to a "
|
||||
"detached helper (refresh ran inside the gateway process tree)"
|
||||
"transient launchd job (refresh ran inside the gateway process tree)"
|
||||
)
|
||||
return True
|
||||
|
||||
|
|
|
|||
|
|
@ -735,12 +735,21 @@ class TestLaunchdServiceRecovery:
|
|||
assert "--replace" in plist_path.read_text(encoding="utf-8")
|
||||
# No DIRECT bootout/bootstrap ran (those would kill us mid-sequence).
|
||||
assert not [c for c in run_calls if "bootout" in c or "bootstrap" in c]
|
||||
# Exactly one detached helper was spawned, in a new session, and it
|
||||
# performs both bootout and bootstrap.
|
||||
# Exactly one Popen call was made for the transient launchd job.
|
||||
assert len(popen_calls) == 1
|
||||
cmd, kwargs = popen_calls[0]
|
||||
assert kwargs.get("start_new_session") is True
|
||||
script = cmd[-1]
|
||||
# Must use `launchctl submit` (not `start_new_session=True`) so the
|
||||
# helper runs as a transient launchd job outside the gateway's process
|
||||
# coalition, surviving bootout (#69098).
|
||||
assert cmd[:3] == ["launchctl", "submit", "-l"]
|
||||
assert kwargs.get("start_new_session") is not True
|
||||
assert "-o" in cmd
|
||||
assert "-e" in cmd
|
||||
# The script is passed via -- /bin/bash -c ...
|
||||
bash_idx = cmd.index("--") + 1
|
||||
assert cmd[bash_idx] == "/bin/bash"
|
||||
assert cmd[bash_idx + 1] == "-c"
|
||||
script = cmd[bash_idx + 2]
|
||||
assert "bootout" in script and "bootstrap" in script
|
||||
assert str(plist_path) in script
|
||||
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue