diff options
| -rw-r--r-- | docs/PAGING_FINDINGS.md | 37 | ||||
| -rw-r--r-- | idatui/__init__.py | 2 | ||||
| -rw-r--r-- | idatui/client.py | 79 | ||||
| -rw-r--r-- | tests/test_keepalive.py | 76 |
4 files changed, 186 insertions, 8 deletions
diff --git a/docs/PAGING_FINDINGS.md b/docs/PAGING_FINDINGS.md index 33cb2e6..3d3ec1a 100644 --- a/docs/PAGING_FINDINGS.md +++ b/docs/PAGING_FINDINGS.md @@ -93,15 +93,36 @@ fall back to the disassembly view or show an error panel. Normal decompile bodies are server-truncated with a `[N chars total]` marker (still to be solved for full-body display — see Phase 2). -## Workers self-exit when idle (`idle_ttl_sec`, default 600s) +## Workers self-exit when idle (`idle_ttl_sec`, default 600s) — SOLVED -A headless idalib worker self-exits after ~10 min idle. Its session then lingers -in `idb_list` but calls fail with `"Worker for session <id> is not reachable"` -(distinct from `"Session not found"`). Recovery differs: this needs a fresh -`idb_open` of the same `input_path` (new worker), not just a db re-resolve. The -TUI should (a) raise `idle_ttl_sec` when opening, and/or (b) detect -"not reachable" and transparently re-open. Currently surfaced as a clean -`IDAToolError`; auto-reopen is a TODO for the app layer. +### Why it happens +Each idalib worker runs a `WorkerLifecycle` watchdog +(`ida_pro_mcp/worker_lifecycle.py`): if no JSON-RPC request hits its dispatcher +for `idle_ttl_sec` (default **600s**), the worker **self-exits cleanly**. It is +refcount-free — "activity == alive": every forwarded tool call `touch()`es the +timer. This is deliberate resource hygiene for a *shared* supervisor (each worker +holds an IDA license slot + the DB's RAM; default max 4 workers), aimed at +ephemeral/agent usage that opens a DB and wanders off. For an **interactive TUI +that should be able to just chill, it's the wrong default.** The startup binary +passed to `idalib-mcp` on the CLI always gets 600s (no CLI flag to change it). + +After self-exit the session lingers in `idb_list` but calls fail with +`"Worker for session <id> is not reachable"` (distinct from `"Session not +found"`). + +### The fix (both implemented + verified in `tests/test_keepalive.py`) +1. **`IDAClient.bump_idle_ttl(idle_ttl_sec=1e9)`** — `idb_open` on the + already-open path is idempotent and re-applies the TTL; `set_idle_ttl` has no + upper cap, so ~1e9s is effectively 'never'. This is the intended knob and the + primary fix: the TUI calls it once at startup. +2. **`IDAClient.keepalive(interval=120)` -> `KeepAlive`** — a background + heartbeat issuing a cheap `server_health` (~2ms) that resets the watchdog. + Belt-and-suspenders, and it also covers *adopted* sessions whose path we don't + control / don't want to re-open. + +Recommended TUI policy: `bump_idle_ttl()` on open **and** run a `KeepAlive` as a +safety net. "Worker not reachable" then only occurs on a real crash, which the +app handles by re-`idb_open`. ## Writable path requirement (operational) diff --git a/idatui/__init__.py b/idatui/__init__.py index 8dd371c..24f3c0f 100644 --- a/idatui/__init__.py +++ b/idatui/__init__.py @@ -10,6 +10,7 @@ from .client import ( IDAToolError, IDASessionError, Session, + KeepAlive, ) from .domain import ( Program, @@ -42,4 +43,5 @@ __all__ = [ "IDAToolError", "IDASessionError", "Session", + "KeepAlive", ] diff --git a/idatui/client.py b/idatui/client.py index 44f5cc9..3e5edb4 100644 --- a/idatui/client.py +++ b/idatui/client.py @@ -522,6 +522,35 @@ class IDAClient: def db(self) -> str | None: return self._db + def _session_input_path(self, db: str) -> str: + for s in self.list_sessions(): + if s.session_id == db: + if not s.input_path: + raise IDASessionError(f"session {db} has no input_path to re-open") + return s.input_path + raise IDASessionError(f"session {db} not found") + + def bump_idle_ttl(self, idle_ttl_sec: int = 1_000_000_000, + path: str | None = None) -> None: + """Raise the worker's idle self-exit TTL so an interactive session never + gets reaped while the user is just reading. + + Headless idalib workers self-exit after ``idle_ttl_sec`` (default 600s) + with no requests. ``idb_open`` on the already-open path is idempotent + (returns the same session) and re-applies the TTL, so this simply opens + the pinned session's path with a huge TTL. The default (~31 years) is + effectively 'never'. + """ + p = path or self._session_input_path(self.resolve_db()) + self.call("idb_open", input_path=p, idle_ttl_sec=int(idle_ttl_sec)) + + def keepalive(self, interval: float = 120.0) -> "KeepAlive": + """Return a (not-yet-started) heartbeat that touches the worker's idle + watchdog every ``interval`` seconds. Belt-and-suspenders next to + :meth:`bump_idle_ttl`; also covers adopted sessions we didn't open. + """ + return KeepAlive(self, interval=interval) + # -- convenience ------------------------------------------------------- # def health(self) -> dict: return self.call("server_health") @@ -542,6 +571,56 @@ class IDAClient: raise IDAError(f"tool not found: {tool}") +class KeepAlive: + """Background heartbeat that keeps an idalib worker from idling out. + + Any forwarded request resets the worker's idle timer, so a periodic cheap + ``server_health`` is enough. Failures are swallowed (the next real call will + auto-recover); the heartbeat just keeps a chilling TUI's worker alive. + """ + + def __init__(self, client: "IDAClient", interval: float = 120.0): + if interval <= 0: + raise ValueError("interval must be > 0") + self._client = client + self.interval = interval + self._stop = threading.Event() + self._thread: threading.Thread | None = None + self.beats = 0 + self.failures = 0 + + def start(self) -> "KeepAlive": + if self._thread is not None: + return self + self._stop.clear() + self._thread = threading.Thread( + target=self._run, daemon=True, name="idatui-keepalive" + ) + self._thread.start() + return self + + def stop(self) -> None: + self._stop.set() + t = self._thread + self._thread = None + if t is not None: + t.join(timeout=2.0) + + def __enter__(self) -> "KeepAlive": + return self.start() + + def __exit__(self, *exc) -> None: + self.stop() + + def _run(self) -> None: + while not self._stop.wait(self.interval): + try: + self._client.health() + self.beats += 1 + except IDAError: + self.failures += 1 + + def _is_stale_session(e: IDAToolError) -> bool: msg = e.message.lower() return any(marker in msg for marker in _STALE_SESSION_MARKERS) diff --git a/tests/test_keepalive.py b/tests/test_keepalive.py new file mode 100644 index 0000000..2910e1a --- /dev/null +++ b/tests/test_keepalive.py @@ -0,0 +1,76 @@ +#!/usr/bin/env python3 +"""Prove that an interactive session can 'just chill' without the worker idling +out. Uses a deliberately short worker TTL to keep the test fast. + + python3 tests/test_keepalive.py + +Needs ~/ida-venv + a running server (tests/serverctl.sh) and a writable target. +""" +import os +import sys +import time + +HERE = os.path.dirname(os.path.abspath(__file__)) +REPO = os.path.dirname(HERE) +sys.path.insert(0, REPO) +from idatui.client import IDAClient, IDAToolError # noqa: E402 + +PASS = FAIL = 0 + + +def check(name, cond, detail=""): + global PASS, FAIL + if cond: + PASS += 1 + print(f" ok {name}") + else: + FAIL += 1 + print(f" FAIL {name} {detail}") + + +def alive(c, sid): + try: + c.set_db(sid) + c.health() + return True + except IDAToolError: + return False + + +def main(): + target = os.path.join(REPO, "targets", "ls_ttl") + if not os.path.exists(target): + src = os.path.join(REPO, "bin", "ls") + os.makedirs(os.path.dirname(target), exist_ok=True) + import shutil + shutil.copy(src, target) + + c = IDAClient(timeout=300) + c.connect() + SHORT = 12 # worker self-exits after ~12s idle unless kept alive + + print("[heartbeat keeps a short-TTL worker alive]") + sid = c.call("idb_open", input_path=target, idle_ttl_sec=SHORT)["session"]["session_id"] + c.set_db(sid) + ka = c.keepalive(interval=4.0).start() + time.sleep(SHORT * 2 + 2) # idle well past the TTL, but heartbeat is beating + ok = alive(c, sid) + check("alive past 2x TTL with heartbeat", ok, f"beats={ka.beats}") + check("heartbeat actually beat", ka.beats >= 4, f"beats={ka.beats}") + check("heartbeat had no failures", ka.failures == 0, f"failures={ka.failures}") + ka.stop() + + print("\n[bump_idle_ttl makes it effectively immortal]") + sid = c.call("idb_open", input_path=target, idle_ttl_sec=SHORT)["session"]["session_id"] + c.set_db(sid) + c.bump_idle_ttl() # ~1e9 seconds + time.sleep(SHORT * 2 + 6) # zero requests during this window + check("alive past 2x TTL after bump, zero requests", alive(c, sid)) + + c.close() + print(f"\n{PASS} passed, {FAIL} failed") + return 1 if FAIL else 0 + + +if __name__ == "__main__": + raise SystemExit(main()) |