fix: resident Ghidra bridge empties decompiles over time (DecompInterface leak + reused TaskMonitor)

[branover]

What

On a long-lived resident Ghidra bridge (re_bridge_start), re_decompile_at / re_decompile_function begin returning empty pseudo-C (function signature + callee list, no body) for many functions after the bridge has served for hours. Two independent container-side defects in the bridge's pyghidra server cause it — both masked on the headless path (one process per call, so OS teardown reclaims everything) and surfacing only on the long-lived resident bridge.

① Undisposed DecompInterface — the dominant cause

_focus_facts (and taint_core) built a DecompInterface per call and never dispose()d it. Each one spawns a native decompile subprocess + I/O threads, so a resident server accumulates them until pthread_create fails (EAGAIN) and decompileFunction can no longer start its subprocess → decompileCompleted()==false → empty body. Observed live in a bridge container's log: Failed to start thread ... pthread_create failed (EAGAIN) at ~15.6 h uptime.

Fix: dispose() in a finally (per request in _focus_facts) / before return (taint_core reuses one interface across its candidate loop). The emulator path already disposed (emu.dispose()), so this was an oversight.

② Reused ConsoleTaskMonitor — a second, latent cause

The probe minted one monitor and serve_bridge reused it for every RPC. DecompInterface.decompileFunction(f, 60, monitor) cancels the monitor it is handed when the 60 s timeout elapses, and a ConsoleTaskMonitor stays cancelled — so a single slow function poisons every later decompile into an empty body.

Fix: mint a fresh monitor per request — serve_bridge/_serve_one take a make_monitor factory; the probe passes the ConsoleTaskMonitor class.

Why it isn't the content hash / cache

The empty body is a fresh failed decompile, not a stale cache read: the decompile path (agent_tools._decomp) always returns the live decompiler result and never reads back from the Observation store. The identical content_hash across a target's observations is by design — it is the target's sha256 (content_hash_for), scoping observations to the exact bytes; dedup additionally keys on tool + args + result_kind, so it is not defeated.

Verification

• Added offline regression tests in test_pyghidra_bridge.py: a fresh-monitor-per-request check and two DecompInterface-dispose checks (success + error path). All three fail on the old code, pass on the fix.
• Full offline fast tier green (pytest -m "not slow"): 1652 passed, 6 skipped.
• The Ghidra-touching decompile core is additionally exercised end-to-end in the WITH_GHIDRA lane.

Notes

• Probes are mounted at runtime, so no image rebuild is needed — but an already-running resident bridge must be restarted (re_bridge_stop → re_bridge_start, which re-opens the warm slot, not a cold re-analysis) to load the patched probe and clear an exhausted/poisoned server.
• The sqlite3.OperationalError: database is locked occasionally seen under parallel decompile_at is unrelated host-side SQLite single-writer contention (swallowed best-effort by _record_obs); not addressed here.

🤖 Generated with Claude Code

[branover]

Independent merge-gate review — PR #271

Reviewer: independent pr-reviewer agent (not the author). Ran /code-review high and /security-review over git diff origin/main...HEAD (head 0070b33).

Verdict: APPROVE (with two non-blocking Low findings for discussion)

Both fixes are correct and the three added tests are genuine regressions (I verified they fail on the pre-fix source and pass on the new source). The two DecompInterface construction sites in the managed bridge are the only ones, and both are now disposed. No security invariant is weakened. No model/UI change, so no migration or ux-contract update is required — correct.

---

Correctness — both fixes verified

1. _focus_facts dispose-in-finally is safe. The focus dict (pyghidra_lib.py:409-418) is assembled entirely from extracted primitives (str/int/list-of-dict-of-str) inside the try body before return focus. Python evaluates the return expression, then runs the finally, then returns — so deci.dispose() (line 428-429) runs after every read off res/hf/df/target has completed and the return value is captured. No live Java ref is touched post-dispose. Correct.
2. taint_core single pre-return dispose is adequate for the success path. One interface reused across the candidates loop, disposed once before the normal return (line 660-661); all hf/res reads finish inside the loop. Correct on the happy path (but see Finding 1).
3. Monitor factory is correct. make_monitor() is invoked exactly once per request, at the single bridge_dispatch(...) call site in _serve_one (line 1199). Passing the ConsoleTaskMonitor class as a 0-arg factory (ghidra_bridge_probe.py:47) mints a fresh, un-cancelled monitor per request — the fix. The two pre-existing _serve_one tests were correctly updated to pass lambda: None.

Missed-leak-site sweep — clean

The only two DecompInterface sites in the managed bridge are _focus_facts and taint_core; both are now disposed. xrefs_core/search/list/ping/rename construct no DecompInterface, and emulate_core already disposes its EmulatorHelper. The fresh-monitor change is complete: the single shared-monitor path (_serve_one → bridge_dispatch) is the only one, and it now mints per request. The _RemoteOps.remote_decompile path in engine/re/ghidra_bridge.py:118-128 also leaks a DecompInterface via remote_eval, but that is the researcher's own external Ghidra bridge (a different seam), pre-existing and not in this diff — out of scope, noted for a possible follow-up.

Security invariants — all intact

Container-side probe code only. Bind is unchanged (0.0.0.0 inside the --network none sandbox — pre-existing, host-loopback guarantee holds at the publish boundary). No secret is logged/stored/returned; no policy-seam touch; no new deserialization/injection surface; make_monitor() takes no untrusted input. /security-review surfaced no HIGH/MEDIUM findings.

Findings (both Low, non-blocking)

Finding 1 [Low] — taint_core dispose is not in a finally (unlike the _focus_facts fix in the same PR). pyghidra_lib.py:660-661. The pre-return deci.dispose() is skipped if anything in the candidate loop raises. list(hf.getPcodeOps()) (line 565), the fixpoint loop (580-617), and the sink-scan loop (624-655) call raw Java methods without contextlib.suppress, so a JPype/HighFunction error there raises past the dispose and re-introduces the exact leak this PR fixes — just on the error path, one-per-taint-op rather than per-request. bridge_dispatch catches the exception but does not dispose. Recommend wrapping the loop + return in try: ... finally: with contextlib.suppress(Exception): deci.dispose(), matching the _focus_facts shape. Low severity: taint ops are far rarer than decompiles and each leaks at most one interface per failed op, so real-world accumulation is slow — but it's the same class of defect and cheap to make consistent.

Finding 2 [Low] — taint_core dispose is untested offline. The PR adds two _focus_facts dispose tests but none for taint_core, so the second dispose site (and its error-path gap in Finding 1) isn't locked in. A small offline test paralleling test_focus_facts_disposes_the_decompiler (fake DecompInterface, assert disposed == 1, plus an error-path variant once Finding 1 is addressed) would cover it. Non-blocking.

Test quality — strong

The three new tests are genuine regressions (verified: all three FAIL on the pre-fix source, PASS on the new source). The offline sys.modules fake-ghidra injection is sound — monkeypatch.setitem auto-restores each entry on teardown, so no cross-test pollution, and it correctly targets _focus_facts's function-local from ghidra.app.decompiler import DecompInterface. The error-path test asserts dispose still runs via the finally. The one gap is Finding 2.

---

This verdict lives in the comment body per the single-owner-repo gate (GitHub refuses --approve/--request-changes on the owner's own PR). Findings are non-blocking; leaving them for the author to address or discuss rather than fixing in-place.

[branover]

Review findings addressed — f71b1f5

Both non-blocking findings are fixed:

① taint_core dispose now in a finally. Wrapped the candidate loop + return in try/finally and moved deci.openProgram(program) inside the try, matching _focus_facts, so an unsuppressed Java error in the candidate loop (or in openProgram) frees the interface on the error path instead of leaking it. The re-indent is logic-preserving — git diff -w shows only the try/finally wrapper and the openProgram/return move, no loop-body change.

② Offline coverage for the taint dispose. Added test_taint_core_disposes_the_decompiler (success path) and test_taint_core_disposes_even_when_it_raises — the latter locks in the finding: it fails on the pre-finally (bare pre-return) dispose and passes on the fix. Bridge suite is 29 passed; full offline fast tier green (1654 passed, 6 skipped).

Re: the pre-existing _RemoteOps._decompile_one leak you flagged (engine/re/ghidra_bridge.py) — confirmed real, but it's the researcher-bridge (connect_ops/jfx_bridge) seam rather than the managed resident bridge, and pre-existing, so I've left it out of this PR to keep scope tight and tracked it as a separate follow-up.