diff --git a/declib/api/server_registry.py b/declib/api/server_registry.py index 5ff6ac8..100c4f6 100644 --- a/declib/api/server_registry.py +++ b/declib/api/server_registry.py @@ -72,6 +72,83 @@ def unregister_server(server_id: str) -> bool: return False +# A pruned record is the only evidence that a server ever existed. Deleting it +# outright means the *next* command can explain what happened but every command +# after that cannot -- and the command agents run most is `list`, which prunes. +# Measured on a 40-binary benchmark: only 10 of 79 "missing server" errors +# carried the useful diagnosis; 69 had already had the corpse eaten by an +# intervening `list`. So pruning leaves a tombstone behind instead. +_TOMBSTONE_SUFFIX = ".dead.json" +_TOMBSTONE_KEEP = 16 + + +def _tombstone_path(server_id: str) -> Path: + return _registry_dir() / f"{server_id}{_TOMBSTONE_SUFFIX}" + + +def _write_tombstone(record: Dict) -> None: + """Record that a server died, so later commands can still say so.""" + server_id = record.get("id") + if not server_id: + return + payload = dict(record) + payload["died_at"] = time.time() + try: + path = _tombstone_path(str(server_id)) + tmp_path = path.with_suffix(".tmp") + with open(tmp_path, "w") as f: + json.dump(payload, f, indent=2, default=str) + os.replace(tmp_path, path) + except Exception as exc: + _l.debug("Failed to write tombstone for %s: %s", server_id, exc) + return + _trim_tombstones() + + +def _trim_tombstones() -> None: + """Keep only the most recent tombstones; they are a hint, not a log.""" + try: + stones = sorted( + _registry_dir().glob(f"*{_TOMBSTONE_SUFFIX}"), + key=lambda p: p.stat().st_mtime, + reverse=True, + ) + except Exception: + return + for old in stones[_TOMBSTONE_KEEP:]: + try: + old.unlink() + except Exception: + pass + + +def list_tombstones() -> List[Dict]: + """Recently-died servers, newest first.""" + out: List[Dict] = [] + try: + stones = sorted(_registry_dir().glob(f"*{_TOMBSTONE_SUFFIX}")) + except FileNotFoundError: + return [] + for entry in stones: + try: + with open(entry, "r") as f: + out.append(json.load(f)) + except Exception: + continue + out.sort(key=lambda r: r.get("died_at") or 0, reverse=True) + return out + + +def clear_tombstone(server_id: str) -> None: + """Forget a death -- called when a binary is successfully reloaded.""" + try: + _tombstone_path(str(server_id)).unlink() + except FileNotFoundError: + pass + except Exception: + pass + + def _is_record_live(record: Dict) -> bool: pid = record.get("pid") socket_path = record.get("socket_path") @@ -96,7 +173,10 @@ def list_servers(prune_stale: bool = True, pruned: Optional[List[Dict]] = None) """ records: List[Dict] = [] try: - entries = sorted(_registry_dir().glob("*.json")) + entries = sorted( + e for e in _registry_dir().glob("*.json") + if not e.name.endswith(_TOMBSTONE_SUFFIX) + ) except FileNotFoundError: return [] @@ -111,6 +191,7 @@ def list_servers(prune_stale: bool = True, pruned: Optional[List[Dict]] = None) if prune_stale and not _is_record_live(record): if pruned is not None: pruned.append(record) + _write_tombstone(record) try: entry.unlink() except FileNotFoundError: diff --git a/declib/cli/decompiler_cli.py b/declib/cli/decompiler_cli.py index 503dde5..6b23aef 100644 --- a/declib/cli/decompiler_cli.py +++ b/declib/cli/decompiler_cli.py @@ -213,6 +213,16 @@ def _select_server( if (not server_id or r.get("id") == server_id) and (not backend or r.get("backend") == backend) ] + # `reaped` only holds servers *this* call pruned. Whichever command ran + # first after the death already pruned it -- usually `list` -- so for + # every later command the corpse is only in the tombstones. + if not dead: + dead = [ + r for r in server_registry.list_tombstones() + if (not server_id or r.get("id") == server_id) + and (not backend or r.get("backend") == backend) + and (not binary_path or r.get("binary_path") == binary_path) + ] if dead: corpse = dead[0] binary = corpse.get("binary_path") or "" @@ -600,6 +610,10 @@ def cmd_load(args) -> int: timeout=args.timeout, backend=backend, ) + # The binary is usable again; stop reporting the old death. + for stone in server_registry.list_tombstones(): + if stone.get("binary_path") == str(binary_path): + server_registry.clear_tombstone(str(stone.get("id"))) _emit(args, { "status": "started", "id": record["id"], @@ -642,6 +656,7 @@ def cmd_list(args) -> int: return 0 if not records: print(f"No running decompiler servers. (registry: {registry_dir})") + _report_recent_deaths() return 0 print(f"{'ID':<12} {'BACKEND':<8} {'PID':<8} BINARY") for r in records: @@ -650,6 +665,20 @@ def cmd_list(args) -> int: return 0 + +def _report_recent_deaths() -> None: + """Tell the caller a server died, rather than letting it read as never-started.""" + stones = server_registry.list_tombstones() + if not stones: + return + print("\nRecently died (analysis is gone; reload to continue):") + for stone in stones[:5]: + binary = stone.get("binary_path") or "" + cmd = f"decompiler load {binary}" + if stone.get("backend"): + cmd += f" --backend {stone['backend']}" + print(f" {stone.get('id','?'):<12} {binary}\n reload: {cmd}") + def _stop_server_by_record(record: Dict, save_mode: Optional[str] = None) -> bool: """Shut down the server process backing `record`. diff --git a/tests/test_decompiler_cli.py b/tests/test_decompiler_cli.py index c897c57..678b319 100644 --- a/tests/test_decompiler_cli.py +++ b/tests/test_decompiler_cli.py @@ -2525,6 +2525,83 @@ def test_pruned_records_are_reported(self): self.assertEqual(reaped[0]["binary_path"], "/tmp/gone") finally: os.environ.pop("DECLIB_SERVER_REGISTRY", None) +class TestServerTombstones(unittest.TestCase): + """A death must stay explainable after the corpse is pruned. + + `list` prunes, and `list` is the command agents run most, so without a + tombstone the useful "your server died" diagnosis is destroyed by the very + command used to check state -- and every later command reads as though no + server was ever started. + """ + + def setUp(self): + import tempfile + self._tmp = tempfile.TemporaryDirectory() + os.environ["DECLIB_SERVER_REGISTRY"] = self._tmp.name + + def tearDown(self): + os.environ.pop("DECLIB_SERVER_REGISTRY", None) + self._tmp.cleanup() + + def _dead_record(self): + from declib.api import server_registry + server_registry.register_server({ + "id": "deadbeef01", "socket_path": "/nonexistent/x.sock", + "binary_path": "/tmp/gone", "backend": "ida", "pid": 999999, + }) + + def test_pruning_leaves_a_tombstone(self): + from declib.api import server_registry + + self._dead_record() + self.assertEqual(server_registry.list_servers(), []) + stones = server_registry.list_tombstones() + self.assertEqual([s["id"] for s in stones], ["deadbeef01"]) + self.assertEqual(stones[0]["binary_path"], "/tmp/gone") + self.assertIn("died_at", stones[0]) + + def test_diagnosis_survives_a_second_lookup(self): + """The regression: the first call pruned, so the second went blind.""" + from declib.api import server_registry + + self._dead_record() + server_registry.list_servers() # first call eats the corpse + stones = server_registry.list_tombstones() + self.assertTrue(stones, "second lookup must still be able to explain") + + def test_tombstones_are_not_mistaken_for_live_records(self): + from declib.api import server_registry + + self._dead_record() + server_registry.list_servers() + # The tombstone is a *.json file in the same directory; it must never + # come back as a server. + self.assertEqual(server_registry.list_servers(), []) + self.assertIsNone(server_registry.find_server(server_id="deadbeef01")) + + def test_reload_clears_the_tombstone(self): + from declib.api import server_registry + + self._dead_record() + server_registry.list_servers() + self.assertTrue(server_registry.list_tombstones()) + server_registry.clear_tombstone("deadbeef01") + self.assertEqual(server_registry.list_tombstones(), []) + + def test_tombstones_are_capped(self): + """A hint, not a log -- they must not grow without bound.""" + from declib.api import server_registry + + for i in range(server_registry._TOMBSTONE_KEEP + 10): + server_registry.register_server({ + "id": f"corpse{i:04d}", "socket_path": "/nonexistent/x.sock", + "binary_path": f"/tmp/gone{i}", "backend": "ida", "pid": 999999, + }) + server_registry.list_servers() + self.assertLessEqual(len(server_registry.list_tombstones()), + server_registry._TOMBSTONE_KEEP) + + class TestDisassembleRangeArgs(unittest.TestCase): """Range-disassembly plumbing that needs no backend or fixture binary."""