From 3a350508f7c15cdd969cb18fc4d227028df30b6d Mon Sep 17 00:00:00 2001 From: Sora-bluesky Date: Thu, 10 Sep 2026 06:49:56 +0900 Subject: [PATCH] fix(cli): wait for SessionDB teardown when deleting a profile close_all_under returned after the last release dropped the generation and before the physical close finished, so rmtree still saw the open handle. Wait directory-matching teardown barriers the same way close_all does. --- hermes_state_registry.py | 17 +++++--- .../test_shared_session_db_registry.py | 42 +++++++++++++++++++ 2 files changed, 53 insertions(+), 6 deletions(-) diff --git a/hermes_state_registry.py b/hermes_state_registry.py index 3d94c80a32..5b0b594ee8 100644 --- a/hermes_state_registry.py +++ b/hermes_state_registry.py @@ -22,8 +22,8 @@ Lifecycle rules: generation is published while the previous generation is still tearing down. - A path can have SEVERAL closes admitted at once (the current generation's final release plus a retired generation's drain). The path barrier COUNTS them and is lifted only by - the last one to settle, so neither ``acquire`` nor ``close_all`` can escape while any - handle for that path is still inside checkpoint/WAL-unlink. + the last one to settle, so neither ``acquire`` nor ``close_all`` / ``close_all_under`` + can escape while any handle for that path is still inside checkpoint/WAL-unlink. - Maintenance callers borrow handles with a temporary registry reference instead of iterating an unpinned snapshot. """ @@ -365,6 +365,10 @@ def close_all_under(directory: str | Path) -> int: Profile delete rmtree (and a same-name recreate) fails while this process still holds ``state.db``. Same contract as ``MemoryStore.release_all_under``: a live holder is expected to fail afterward; a process that holds none is a no-op returning 0. + + A final ``release()`` can drop the generation and admit teardown before the physical + close finishes. Wait for those directory-matching barriers even when no generation + remains, otherwise rmtree still sees the open handle. """ try: root = Path(directory).expanduser().resolve() @@ -377,12 +381,13 @@ def close_all_under(directory: str | Path) -> int: for generation in list(_generations.values()) + list(_retired.values()) if _path_is_under(generation.path, root) ] - if not generations: - return 0 - selected_paths = {generation.path for generation in generations} + # Collect by directory, not by remaining generations: a last release already + # popped the generation and left only ``_tearing_down``. active_teardowns = [ - barrier for path, barrier in _tearing_down.items() if path in selected_paths + barrier for path, barrier in _tearing_down.items() + if _path_is_under(path, root) ] + selected_paths = {generation.path for generation in generations} for path in selected_paths: teardown_barriers[path] = _admit_teardown_locked(path) for generation in generations: diff --git a/tests/hermes_state/test_shared_session_db_registry.py b/tests/hermes_state/test_shared_session_db_registry.py index 8c421603a4..caf078ad6b 100644 --- a/tests/hermes_state/test_shared_session_db_registry.py +++ b/tests/hermes_state/test_shared_session_db_registry.py @@ -671,3 +671,45 @@ class TestCloseAllUnder: profile_dir = tmp_path / "profiles" / "empty" profile_dir.mkdir(parents=True) assert registry.close_all_under(profile_dir) == 0 + + def test_waits_for_admitted_teardown_after_generation_is_gone(self, tmp_path, monkeypatch): + """Final release admits teardown before close; rmtree still needs that wait.""" + profile_dir = tmp_path / "profiles" / "work" + profile_dir.mkdir(parents=True) + db = registry.acquire(profile_dir / "state.db") + entered, resume = TestMultiGenerationTeardownBarrier._pause_teardown_of( + monkeypatch, db + ) + errors: list[BaseException] = [] + releaser = TestMultiGenerationTeardownBarrier._release_async(db, errors) + try: + assert entered.wait(5.0) + resolved = (profile_dir / "state.db").resolve() + assert registry._generations.get(resolved) is None + barrier = registry._tearing_down.get(resolved) + assert barrier is not None and not barrier.event.is_set() + + swept: list[int] = [] + sweep_done = threading.Event() + + def _sweep() -> None: + swept.append(registry.close_all_under(profile_dir)) + sweep_done.set() + + sweeper = threading.Thread(target=_sweep, daemon=True) + sweeper.start() + assert not sweep_done.wait(0.5), ( + "close_all_under returned with a close still pending" + ) + assert db._conn is not None + + resume.set() + assert sweep_done.wait(10.0) + sweeper.join(10.0) + finally: + resume.set() + releaser.join(10.0) + + assert errors == [] + assert db._conn is None + assert swept == [0]