diff --git a/hermes_cli/web_server.py b/hermes_cli/web_server.py index 9e7bf8421abe7..857f89605ba24 100644 --- a/hermes_cli/web_server.py +++ b/hermes_cli/web_server.py @@ -2911,6 +2911,48 @@ def _collect_profile_gateway_topology() -> Dict[str, Any]: return {"profiles": profile_names, "gateway_mode": mode, "gateways": gateways} +# /api/status is polled ~1/s by the desktop app while it waits for the backend +# (and again by the dashboard badge). Each uncached call above walks 7+ profile +# homes (yaml.safe_load with the pure-Python loader + psutil process-table +# probes + realpath walks) inside the default executor; concurrent polls pile +# up and hold the GIL for 14-16s, starving the event loop — the desktop WS +# never receives gateway.ready and boot fails ("event loop stalled ... GIL +# pressure suspected"). Topology changes on gateway start/stop, so a short TTL +# cache with a collapse lock keeps the scan to one per window. The cache also +# remembers which collector produced the entry: tests monkeypatch +# _collect_profile_gateway_topology per case, and the identity check keeps +# them hermetic without needing a reset hook (a swapped collector is a miss). +_TOPOLOGY_CACHE: Dict[str, Any] = {"ts": 0.0, "data": None, "fn": None} +_TOPOLOGY_CACHE_LOCK = threading.Lock() +_TOPOLOGY_CACHE_TTL = 10.0 + + +def _topology_cache_get(fn: Any) -> Optional[Dict[str, Any]]: + if ( + _TOPOLOGY_CACHE["data"] is not None + and _TOPOLOGY_CACHE["fn"] is fn + and time.monotonic() - _TOPOLOGY_CACHE["ts"] < _TOPOLOGY_CACHE_TTL + ): + return _TOPOLOGY_CACHE["data"] + return None + + +def _collect_profile_gateway_topology_cached() -> Dict[str, Any]: + fn = _collect_profile_gateway_topology + cached = _topology_cache_get(fn) + if cached is not None: + return cached + with _TOPOLOGY_CACHE_LOCK: + cached = _topology_cache_get(fn) + if cached is not None: + return cached + data = fn() + _TOPOLOGY_CACHE["data"] = data + _TOPOLOGY_CACHE["fn"] = fn + _TOPOLOGY_CACHE["ts"] = time.monotonic() + return data + + @app.get("/api/ssh/ownership") async def get_ssh_ownership(request: Request): _require_token(request) @@ -3237,7 +3279,7 @@ async def get_status(profile: Optional[str] = None): # per-gateway ``gateways[]`` detail carries host ports (deployment # recon), so it stays gated with the host paths / PID below. topology = await asyncio.get_running_loop().run_in_executor( - None, _collect_profile_gateway_topology + None, _collect_profile_gateway_topology_cached ) status["profiles"] = topology["profiles"] status["gateway_mode"] = topology["gateway_mode"] diff --git a/tests/test_web_server_status_topology_cache.py b/tests/test_web_server_status_topology_cache.py new file mode 100644 index 0000000000000..76c4d0be2ed02 --- /dev/null +++ b/tests/test_web_server_status_topology_cache.py @@ -0,0 +1,117 @@ +"""Regression tests for the /api/status profile-topology cache. + +The desktop app polls /api/status ~1/s while waiting for the backend to become +ready. Before the cache, every poll ran a full _collect_profile_gateway_topology +scan (per-profile yaml.safe_load with the pure-Python loader + psutil +process-table probes + realpath walks) in the default executor; on multi-profile +installs the concurrent scans held the GIL for 14-16s and starved the event +loop, so the desktop WS never received gateway.ready and boot escalated to the +"Hermes couldn't start" overlay (#60800). +""" + +import threading +import time + +from hermes_cli import web_server + + +def _reset_cache(): + web_server._TOPOLOGY_CACHE["ts"] = 0.0 + web_server._TOPOLOGY_CACHE["data"] = None + web_server._TOPOLOGY_CACHE["fn"] = None + + +def _fake_topology(calls, delay=0.0): + def _collect(): + if delay: + time.sleep(delay) + calls.append(1) + return {"profiles": ["default"], "gateway_mode": "single", "gateways": []} + + return _collect + + +def test_topology_cache_returns_cached_result_within_ttl(monkeypatch): + calls = [] + monkeypatch.setattr( + web_server, "_collect_profile_gateway_topology", _fake_topology(calls) + ) + _reset_cache() + try: + first = web_server._collect_profile_gateway_topology_cached() + second = web_server._collect_profile_gateway_topology_cached() + finally: + _reset_cache() + + assert len(calls) == 1 + assert first is second + + +def test_topology_cache_rescans_after_ttl(monkeypatch): + calls = [] + monkeypatch.setattr( + web_server, "_collect_profile_gateway_topology", _fake_topology(calls) + ) + _reset_cache() + try: + web_server._collect_profile_gateway_topology_cached() + # Age the cache entry past the TTL instead of sleeping through it. + web_server._TOPOLOGY_CACHE["ts"] -= web_server._TOPOLOGY_CACHE_TTL + 1.0 + web_server._collect_profile_gateway_topology_cached() + finally: + _reset_cache() + + assert len(calls) == 2 + + +def test_topology_cache_collapses_concurrent_scans(monkeypatch): + """Concurrent status polls must not each run their own scan — that pile-up + is exactly the GIL storm the cache exists to prevent.""" + calls = [] + monkeypatch.setattr( + web_server, + "_collect_profile_gateway_topology", + _fake_topology(calls, delay=0.05), + ) + _reset_cache() + results = [] + try: + threads = [ + threading.Thread( + target=lambda: results.append( + web_server._collect_profile_gateway_topology_cached() + ) + ) + for _ in range(8) + ] + for t in threads: + t.start() + for t in threads: + t.join() + finally: + _reset_cache() + + assert len(calls) == 1 + assert len(results) == 8 + assert all(r == results[0] for r in results) +def test_topology_cache_misses_when_collector_is_swapped(monkeypatch): + """Tests (and hot-reload scenarios) monkeypatch the collector; a swapped + function identity must be a cache miss so stale data from the previous + collector never leaks across the swap.""" + calls_a, calls_b = [], [] + monkeypatch.setattr( + web_server, "_collect_profile_gateway_topology", _fake_topology(calls_a) + ) + _reset_cache() + try: + first = web_server._collect_profile_gateway_topology_cached() + monkeypatch.setattr( + web_server, "_collect_profile_gateway_topology", _fake_topology(calls_b) + ) + second = web_server._collect_profile_gateway_topology_cached() + finally: + _reset_cache() + + assert len(calls_a) == 1 + assert len(calls_b) == 1 + assert first is not second