mirror of
https://github.com/NousResearch/hermes-agent.git
synced 2026-11-06 05:17:15 +00:00
perf(browser): the in-call ownership check stops once the daemon can never be ours
Gate (2c, efficiency): the 0.5 s check while browser_exec runs stopped only once this process owned the daemon. On Windows, in a managed worker, or for a daemon another Hermes started it can never be owned, so it ran every tick for the whole call (up to ~600 short threads in a 300 s call; for another Hermes's daemon each tick also read its cmdline, uids, sid and environ). _own_harness_daemon now answers True (owned), False (never ours) or None (nothing to own yet), and the check stops on anything but None. The managed-worker comment now gives the real reason: a killed worker must not take the next turn's browser along; its clean exit still runs main's atexit browser cleanup. Tests: for another Hermes's daemon and in a managed worker the check answers stop; both fail with the old owned-only stop rule.
This commit is contained in:
1 parent
9cb4e058cd
commit
e579e9b98e
2 files changed
+22
-15
No files matched your search
@@ -156,7 +156,8 @@ def test_a_daemon_another_live_hermes_started_on_the_same_name_is_never_register
|
||||
stdin=subprocess.DEVNULL, start_new_session=True, env={**os.environ, **tag})
|
||||
try:
|
||||
_publish(runtime, "bu-default", daemon.pid)
|
||||
browser_use_cli._own_harness_daemon({"BH_HOME": str(runtime.parent)})
|
||||
# The in-call check stops too, instead of re-checking a daemon that is never ours every tick.
|
||||
assert browser_use_cli._own_harness_daemon_until_owned({"BH_HOME": str(runtime.parent)}) is False
|
||||
assert registered == []
|
||||
finally:
|
||||
daemon.kill()
|
||||
@@ -200,6 +201,7 @@ def test_a_managed_worker_never_owns_its_harness_daemon(fake_daemon, registered,
|
||||
_publish(runtime, "bu", proc.pid)
|
||||
monkeypatch.setattr(runtime_session_store, "_worker_process", True)
|
||||
|
||||
browser_use_cli._own_harness_daemon({"BH_RUNTIME_DIR": str(runtime), "BU_NAME": "work"})
|
||||
env = {"BH_RUNTIME_DIR": str(runtime), "BU_NAME": "work"}
|
||||
assert browser_use_cli._own_harness_daemon_until_owned(env) is False # and the in-call check stops
|
||||
|
||||
assert registered == []
|
||||
+18
-13
@@ -649,10 +649,9 @@ def _published_daemon_pid(env: dict) -> Optional[int]:
|
||||
|
||||
def _own_harness_daemon_until_owned(env: dict) -> Optional[bool]:
|
||||
"""Periodic check while browser_exec runs: own the daemon as soon as it publishes its pid, then
|
||||
stop (False)."""
|
||||
_own_harness_daemon(env)
|
||||
with _owned_daemon_groups_lock:
|
||||
return False if daemon_identity(env) in _owned_daemon_groups else None
|
||||
stop (False). It also stops once the daemon can never be ours (Windows, a managed worker, a daemon
|
||||
another Hermes started), instead of re-checking every tick for the whole call."""
|
||||
return None if _own_harness_daemon(env) is None else False
|
||||
|
||||
|
||||
def _reload_daemons(daemons) -> None: # each ``daemon_identity`` daemon, with its own dirs
|
||||
@@ -678,7 +677,7 @@ _owned_daemon_groups: dict[tuple[str, ...], int] = {}
|
||||
_owned_daemon_groups_lock = threading.Lock()
|
||||
|
||||
|
||||
def _own_harness_daemon(env: dict) -> None:
|
||||
def _own_harness_daemon(env: dict) -> Optional[bool]:
|
||||
"""Hand the daemon behind ``env``'s BU_NAME to the owner-death supervisor.
|
||||
|
||||
The harness publishes its daemon pid in its runtime dir (``harness_pid_file``: JSON
|
||||
@@ -686,33 +685,38 @@ def _own_harness_daemon(env: dict) -> None:
|
||||
``browser_harness.daemon`` leading its own session and its spawn tag names this process, so a
|
||||
stale or planted pid file, or a daemon another Hermes started, never reaches the supervisor.
|
||||
Runs after every browser_exec (and while a long one runs, until the daemon is owned): a pid
|
||||
already owned for this daemon costs one file read."""
|
||||
already owned for this daemon costs one file read.
|
||||
|
||||
Returns True once the daemon is owned, False when it can never be (Windows, a managed worker, a
|
||||
daemon another Hermes started), None when there is nothing to own yet."""
|
||||
if os.name != "posix":
|
||||
return # Windows: the harness daemon is outside this process's job; nothing to register.
|
||||
return False # Windows: the harness daemon is outside this process's job; nothing to register.
|
||||
from agent.runtime_session_store import is_worker_process
|
||||
if is_worker_process():
|
||||
return # a managed per-turn worker: the next turn's worker keeps driving this daemon
|
||||
# A managed per-turn worker never owns it, so a killed worker does not take the browser the
|
||||
# next turn's worker drives along (its clean exit still runs the atexit browser cleanup).
|
||||
return False
|
||||
import psutil
|
||||
|
||||
daemon = daemon_identity(env)
|
||||
pid = _published_daemon_pid(env)
|
||||
if pid is None:
|
||||
logger.debug("browser_exec: no harness daemon pid published for %s", daemon[-1])
|
||||
return
|
||||
return None
|
||||
try:
|
||||
with _owned_daemon_groups_lock:
|
||||
if _owned_daemon_groups.get(daemon) == pid:
|
||||
return
|
||||
return True
|
||||
proc = psutil.Process(pid)
|
||||
argv = proc.cmdline()
|
||||
if (os.getsid(pid) != pid or proc.uids().real != os.getuid() # windows-footgun: ok — posix-only, returned above
|
||||
or argv[argv.index("-m") + 1] != "browser_harness.daemon"):
|
||||
return
|
||||
return None
|
||||
if not spawned_by_this_process(pid):
|
||||
return # another Hermes on the same BU_NAME started it: that Hermes owns it
|
||||
return False # another Hermes on the same BU_NAME started it: that Hermes owns it
|
||||
except (OSError, ValueError, IndexError, psutil.Error) as exc:
|
||||
logger.debug("browser_exec: no harness daemon to supervise for %s: %s", daemon[-1], exc)
|
||||
return
|
||||
return None
|
||||
with _owned_daemon_groups_lock:
|
||||
replaced = _owned_daemon_groups.get(daemon)
|
||||
_owned_daemon_groups[daemon] = pid
|
||||
@@ -720,6 +724,7 @@ def _own_harness_daemon(env: dict) -> None:
|
||||
register_child(pid, "browser-harness-daemon")
|
||||
if replaced is not None:
|
||||
release_process_group_once_empty(replaced, timeout=0) # the daemon it replaced; a survivor stays covered
|
||||
return True
|
||||
|
||||
|
||||
def _route_exec(env: dict, session: str, task_id: Optional[str], local: bool, local_sidecar: bool,
|
||||
|
||||
Reference in new issue
Block a user