fix: TrackedEnv drop must close the heed env via prepare_for_closing - #213
Merged
Conversation
heed 0.20's OPENED_ENV cache holds a strong Env clone inside its entry, so dropping the last user-side Env leaves the Arc count at 1 (the entry's own) and mdb_env_close NEVER runs. On POSIX: invisible fd + mmap leak. On Windows: data.mdb/lock.mdb stay locked for the process lifetime — the real cause of serve::tests::index_rm_deletes_db_while_serve_holds_real_lmdb_env failing deterministically (os error 32 through the whole 60s retry budget, LMDB registry empty: the holder is invisible to it), and equally of a real 'codesearch index rm' against a running serve never deleting the DB dir. Diagnosed via minimal serve-free repro: open SharedStores, drop, data.mdb still locked + env_closing_event still Some. TrackedEnv::drop now calls Env::prepare_for_closing() — heed's one real close path (takes the entry's reference out, closes synchronously) — before unregistering our registry slot, preserving the existing slot-ordering invariant, which now actually holds. No codebase path clones a raw heed::Env out of a TrackedEnv, so the wrapper is the last user-side reference. Tests: drop_really_closes_heed_env_and_releases_the_files (registry level), sharedstores_drop_releases_db_dir_for_deletion (production shape). Previously failing acceptance test now passes 3/3 in ~0.6s (was: fail in ~62s); full lib suite 612/612. CHANGELOG [1.3.4] entry added; AGENTS.md LMDB rule extended.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Root cause of the deterministic Windows
index rmfailure (todo board #76)heed 0.20's process-global
OPENED_ENVcache stores its own strongEnvclone inside the entry, so dropping the last user-sideEnvleaves the Arc count at exactly 1 (the entry's own) andmdb_env_closenever runs. On POSIX: invisible fd + mmap leak. On Windows:data.mdb/lock.mdbstay locked for the process lifetime.That is the real cause of
serve::tests::index_rm_deletes_db_while_serve_holds_real_lmdb_envfailing deterministically (os error 32 through the whole 60s retry budget, LMDB registry empty — the holder is invisible to it), and equally of a realcodesearch index rmagainst a running serve never deleting the DB dir. Proven by minimal serve-free repro: openSharedStores→ drop → registry empty BUTenv_closing_eventstillSomeanddata.mdblocked.Fix
TrackedEnv::dropnow callsEnv::prepare_for_closing()— heed's one real close path (takes the entry's reference out, closes synchronously) — before unregistering our registry slot. The existing slot-ordering invariant is preserved and now actually holds. Nothing in the codebase clones a rawheed::Envout of aTrackedEnv, so the wrapper is the last user-side reference.Tests
drop_really_closes_heed_env_and_releases_the_files(registry level — heed'sOPENED_ENVentry must be gone after drop, db dir deletable)sharedstores_drop_releases_db_dir_for_deletion(production shape, serve-free, instant)