diff --git a/CHANGELOG.md b/CHANGELOG.md index d36ae261..b5b64673 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -18,6 +18,8 @@ finalized in place with a date — no renaming/migration step needed. ### Fixed +- **SCIP symbol index: every query/rebuild opened its own LMDB env on `db_path/scip` — overlapping operations failed each other with `LMDB double-open prevented`, then rendered as a frozen red `C#!` in the TUI.** LMDB permits one open env per directory per process, and the C# and TypeScript adapters share that directory, so a watcher-triggered rebuild starting while a lazy `find-refs` call held its env (minutes on large Roslyn workspaces), or any C#↔TS overlap on the same repo, tripped the double-open guard and the loser failed outright. Observed in production as `C# symbol rebuild failed: LMDB double-open prevented … (SCIP(…), opened 35.2s ago)` on an actively-used repo — with the error state then frozen on the TUI for days, because `repo_statuses_lightweight()` prefers the cached status over its on-disk probe and a closed repo has no watcher left to retry a rebuild and flip it back to Ready. Two fixes shipped together. (1) **Shared per-directory env**: all SCIP opens now route through `get_or_open_shared_env` (a Weak-ref cache in `lmdb_registry`), handing every concurrent user the same `Arc` — writers serialise on LMDB's single-writer mutex, readers never block, and the C#/TS adapters share one env (5 named DBs pre-created once per env session). The cache never keeps an env alive, so `remove_repo`'s holder-drain is unaffected: drop of the last Arc still closes and frees the registry slot (strengthened by the `prepare_for_closing` fix above). (2) **Error state lifecycle**: idle eviction and force-reindex close now clear `csharp_index_status`/`csharp_index_error` for the alias, letting the on-disk probe (helper available + index exists → Ready) restore the truth instead of a permanent red `!`. Also hoists the `scip_meta` DB name to `constants::SCIP_META_DB_NAME` (previously a duplicated literal in both adapters). + - **`TrackedEnv` drop never actually closed the heed environment — the real cause of `index rm`'s deterministic Windows failure (os error 32).** heed 0.20's process-global `OPENED_ENV` cache holds a strong `Env` clone inside its entry, so dropping the last user-side `Env` leaves the Arc count at exactly 1 (the entry's own) and `mdb_env_close` never runs. On POSIX that silently leaks an fd + mmap; on Windows it locks `data.mdb`/`lock.mdb` against deletion for the life of the process — which made `serve::tests::index_rm_deletes_db_while_serve_holds_real_lmdb_env` fail deterministically through the whole 60 s retry budget with an *empty* LMDB registry (the holder is invisible to it: the registry tracks `TrackedEnv`s, not heed-internal Arc clones), and would equally block a real `codesearch index rm` against a running serve from ever deleting the DB dir. Diagnosed with a minimal serve-free repro (open `SharedStores` → drop → `data.mdb` still locked, `env_closing_event` still `Some`). `TrackedEnv::drop` now closes via `Env::prepare_for_closing()` — heed's one real close path, which takes the entry's reference out and closes synchronously — before freeing our own registry slot (the existing slot-ordering invariant is preserved and now actually holds). Regression tests pin both levels: `drop_really_closes_heed_env_and_releases_the_files` (registry level: heed's `OPENED_ENV` entry must be gone after drop, db dir deletable) and `sharedstores_drop_releases_db_dir_for_deletion` (production shape, serve-free, instant). The previously failing acceptance test now passes 3/3 in ~0.6 s instead of failing in ~62 s; full lib suite 612/612. ## [1.3.3] - 2026-08-18 diff --git a/src/constants.rs b/src/constants.rs index 02789c66..0df35f22 100644 --- a/src/constants.rs +++ b/src/constants.rs @@ -611,6 +611,9 @@ pub const SCIP_CSHARP_DEBOUNCE_MS: u64 = 60_000; // 60 seconds /// LMDB database name for the SCIP symbols table. pub const SCIP_SYMBOLS_DB_NAME: &str = "scip_symbols"; +/// LMDB database name for the SCIP per-repo metadata table. +pub const SCIP_META_DB_NAME: &str = "scip_meta"; + /// LMDB metadata key for the last rebuild timestamp. pub const SCIP_REBUILD_TIMESTAMP_KEY: &str = "last_rebuild_ts"; diff --git a/src/lmdb_registry.rs b/src/lmdb_registry.rs index b5e842d1..73f218e0 100644 --- a/src/lmdb_registry.rs +++ b/src/lmdb_registry.rs @@ -14,7 +14,7 @@ use dashmap::DashMap; use std::mem::ManuallyDrop; use std::ops::Deref; use std::path::{Path, PathBuf}; -use std::sync::OnceLock; +use std::sync::{Arc, OnceLock, Weak}; use std::time::Instant; use crate::cache::safe_canonicalize; @@ -140,6 +140,77 @@ pub fn open_holders_under(path: &Path) -> Vec { } } +// ── Shared-env cache ──────────────────────────────────────────── + +/// Process-wide cache of LMDB environments that multiple components must use +/// CONCURRENTLY (queries, rebuilds, per-language adapters on the same +/// directory). Holds only [`Weak`] references: the cache never keeps an +/// environment alive, it just hands out the live one when it exists. When the +/// last user drops their `Arc`, the [`TrackedEnv`] drops, its registry slot +/// frees, and the stale cache entry is reaped on the next lookup. +static SHARED_ENVS: OnceLock>> = OnceLock::new(); + +/// Open the environment at `path`, or return the already-open shared instance. +/// +/// LMDB permits exactly one open environment per directory per process, so +/// callers that may overlap in time (a rebuild vs. an in-flight query, the C# +/// vs. TypeScript adapters on the same `db_path/scip`) must not each open +/// their own — the second [`TrackedEnv::open`] trips the double-open guard and +/// one side fails outright. This getter makes the collision impossible: the +/// first caller opens (running `init` once to create the named databases) and +/// everyone else receives a clone of the same `Arc`. +/// +/// `build_opts` configures the [`heed::EnvOpenOptions`] and `init` runs once +/// per environment lifetime, right after the open, before the handle is +/// published to other threads. Writers then serialise on LMDB's own +/// single-writer mutex and readers never block. +/// +/// Both closures run while the cache's shard lock is held: they must not +/// re-enter `get_or_open_shared_env` (a path hashing to the same shard +/// self-deadlocks) and should stay cheap. +/// +/// The env-var lookups inside `build_opts`/`init` run only when the directory +/// is opened for the first time in this process — later override changes do +/// not affect an already-shared environment. +pub fn get_or_open_shared_env( + path: &Path, + description: &str, + build_opts: impl FnOnce(&mut heed::EnvOpenOptions), + init: impl FnOnce(&TrackedEnv) -> Result<()>, +) -> Result> { + let canonical = safe_canonicalize(path) + .with_context(|| format!("Cannot canonicalize LMDB path: {}", path.display()))?; + let cache = SHARED_ENVS.get_or_init(DashMap::new); + + loop { + use dashmap::mapref::entry::Entry; + match cache.entry(canonical.clone()) { + Entry::Occupied(occupied) => { + if let Some(env) = occupied.get().upgrade() { + return Ok(env); + } + // Last Arc dropped but the entry survived — reap and retry so + // the Vacant arm below performs a fresh open. + occupied.remove(); + } + Entry::Vacant(vacant) => { + let mut opts = heed::EnvOpenOptions::new(); + build_opts(&mut opts); + // SAFETY: caller contract — same as `TrackedEnv::open`. The + // registry slot guards against any concurrent direct open. + let tracked = unsafe { TrackedEnv::open(&opts, path, description)? }; + // Run init before publishing: the `?` early-return drops + // `tracked`, freeing the registry slot, so a failed init + // leaves the path openable. + init(&tracked)?; + let env = Arc::new(tracked); + vacant.insert(Arc::downgrade(&env)); + return Ok(env); + } + } + } +} + // ── TrackedEnv wrapper ────────────────────────────────────────── /// Wrapper around [`heed::Env`] that prevents double-open panics. @@ -377,6 +448,104 @@ mod tests { let _env2 = unsafe { TrackedEnv::open(&opts, dir2.path(), "test-2").unwrap() }; } + fn shared_opts(opts: &mut heed::EnvOpenOptions) { + opts.map_size(1024 * 1024).max_dbs(4); + // Every test open carries the baseline flags per the AGENTS.md rule. + unsafe { opts.flags(BASE_ENV_FLAGS) }; + } + + /// Direct-open options IDENTICAL to `shared_opts`. heed refuses to reopen + /// a path with different options (max_dbs included) even after the prior + /// env dropped — the AGENTS.md "same options on every open" rule — so the + /// direct opens in the shared-env tests must not reuse `make_opts` + /// (max_dbs 1). + fn shared_compatible_opts() -> heed::EnvOpenOptions { + let mut opts = heed::EnvOpenOptions::new(); + shared_opts(&mut opts); + opts + } + + /// Two callers of the shared getter receive the SAME environment — this is + /// the property whose absence made a watcher rebuild fail with + /// `LMDB double-open prevented` while a lazy find-refs held its own env. + #[test] + fn shared_env_returns_live_instance_to_concurrent_callers() { + let dir = TempDir::new().unwrap(); + let path = dir.path().to_path_buf(); + + let env1 = get_or_open_shared_env(&path, "shared-1", shared_opts, |_| Ok(())).unwrap(); + let env2 = get_or_open_shared_env(&path, "shared-2", shared_opts, |_| Ok(())).unwrap(); + assert!(Arc::ptr_eq(&env1, &env2)); + + // While the shared env is alive, a DIRECT open on the same path must + // still trip the guard — the shared env genuinely occupies the slot. + let direct = unsafe { TrackedEnv::open(&shared_compatible_opts(), &path, "direct") }; + let err = direct.unwrap_err().to_string(); + assert!(err.contains("double-open prevented")); + assert!(err.contains("shared-1")); + } + + /// The cache holds only weak refs: once the last user drops their Arc the + /// registry slot frees (a direct open succeeds) and the next shared caller + /// transparently reopens. + #[test] + fn shared_env_reopens_after_all_arcs_drop() { + let dir = TempDir::new().unwrap(); + let path = dir.path().to_path_buf(); + + { + let _env = get_or_open_shared_env(&path, "shared-1", shared_opts, |_| Ok(())).unwrap(); + } + + // Slot freed after the last Arc dropped. + { + let _direct = + unsafe { TrackedEnv::open(&shared_compatible_opts(), &path, "direct").unwrap() }; + } + + // And the shared getter opens fresh again. + let _again = get_or_open_shared_env(&path, "shared-2", shared_opts, |_| Ok(())).unwrap(); + } + + /// An `init` failure must not leak the registry slot: the error propagates + /// and the env is dropped, leaving the path openable. + #[test] + fn shared_env_init_failure_frees_slot() { + let dir = TempDir::new().unwrap(); + let path = dir.path().to_path_buf(); + + let result = get_or_open_shared_env(&path, "shared-1", shared_opts, |_| { + Err(anyhow::anyhow!("boom")) + }); + assert!(result.is_err()); + + let _direct = + unsafe { TrackedEnv::open(&shared_compatible_opts(), &path, "direct").unwrap() }; + } + + /// N threads racing the FIRST open all succeed and all hold the same env — + /// no thread sees the double-open error the per-caller open produced. + #[test] + fn shared_env_concurrent_first_open_is_safe() { + let dir = TempDir::new().unwrap(); + let path = Arc::new(dir.path().to_path_buf()); + + let handles: Vec<_> = (0..8) + .map(|_| { + let p = Arc::clone(&path); + std::thread::spawn(move || { + get_or_open_shared_env(&p, "shared-race", shared_opts, |_| Ok(())) + }) + }) + .collect(); + + let envs: Vec<_> = handles + .into_iter() + .map(|h| h.join().expect("thread panicked").expect("open failed")) + .collect(); + assert!(envs.windows(2).all(|w| Arc::ptr_eq(&w[0], &w[1]))); + } + /// `open_holders_under` reports every live env at or under the queried /// path — the precondition check `remove_repo`'s lock-class retry waits /// on. Must see (a) an env whose path IS the queried path and (b) an env diff --git a/src/serve/mod.rs b/src/serve/mod.rs index 873ff0d3..584fe86b 100644 --- a/src/serve/mod.rs +++ b/src/serve/mod.rs @@ -1509,9 +1509,28 @@ impl ServeState { None } + /// Drop cached C# symbol-index status/error for `alias`. + /// + /// `repo_statuses_lightweight()` prefers these cached entries over its + /// on-disk probe, so a cached `Error` outlives the repo itself: a closed + /// repo has no watcher left to retry a rebuild or emit `Succeeded`, and + /// the red `C#!` it causes in the TUI freezes forever (observed on a repo + /// whose rebuild lost a one-shot LMDB double-open race days earlier). + /// Remove the entries letting the probe (helper available + index + /// exists → Ready) restore the on-disk truth. Called from idle eviction + /// and `close_repo` (force-reindex reopen). `remove_repo` deliberately + /// does NOT call this: once the alias is unregistered the entries are + /// display-unreachable (`repo_statuses_lightweight` iterates registered + /// repos only), so a clear there would be dead code. + fn clear_csharp_index_state(&self, alias: &str) { + self.csharp_index_status.remove(alias); + self.csharp_index_error.remove(alias); + } + /// Remove a repo from the DashMap, dropping its stores and releasing /// LMDB file handles. Used before force-reindex reopen. fn close_repo(&self, alias: &str) { + self.clear_csharp_index_state(alias); if self.repos.remove(alias).is_some() { tracing::info!( "Closed repo '{}' (dropped stores, released LMDB handles)", @@ -2945,6 +2964,10 @@ impl ServeState { // deleted (the repo can be re-opened on the next query), so we // don't need to await — the cancelled task drains on its own. self.fsw_tasks.remove(alias); + // Cached C# symbol-index state must not outlive the repo (see + // clear_csharp_index_state): without this, an Error entry frozen + // from a lost double-open race renders red forever. + self.clear_csharp_index_state(alias); match self.repos.remove(alias) { Some((_, RepoState::Write { cancel_token, .. })) => { cancel_token.cancel(); diff --git a/src/serve/tests.rs b/src/serve/tests.rs index 4f0a0505..41fc7690 100644 --- a/src/serve/tests.rs +++ b/src/serve/tests.rs @@ -2261,3 +2261,43 @@ async fn index_rm_deletes_db_while_serve_holds_real_lmdb_env() { "expected a clean Unknown-alias error, got: {err:#}" ); } + +/// A cached C# symbol-index Error must NOT outlive the repo it belongs to. +/// +/// `repo_statuses_lightweight()` prefers the cached entry over its on-disk +/// probe, so an Error left behind by idle eviction renders a red `C#!` in the +/// TUI forever — a closed repo has no watcher left to retry a rebuild and +/// flip the state to Ready. Regression guard for the frozen-`!` fix observed +/// on a repo whose rebuild lost a one-shot LMDB double-open race days +/// earlier. +#[serial_test::serial] +#[test] +fn evicting_idle_repo_clears_frozen_csharp_error_state() { + let _env = crate::testing::EnvRestore::set(&[(crate::constants::REPO_IDLE_TIMEOUT_ENV, "1")]); + let state = ServeState::new(ReposConfig::default(), None); + + // Simulate the poisoned state: an Error + message cached by a failed + // watcher rebuild, and a last-access old enough to be evicted. + state + .csharp_index_status + .insert("frozen".to_string(), CSharpIndexStatus::Error); + state.csharp_index_error.insert( + "frozen".to_string(), + "LMDB double-open prevented".to_string(), + ); + state.last_access.insert( + "frozen".to_string(), + std::time::Instant::now() - std::time::Duration::from_secs(5), + ); + + state.evict_idle_repos(); + + assert!( + !state.csharp_index_status.contains_key("frozen"), + "eviction must clear the cached C# status — a frozen Error renders red forever otherwise" + ); + assert!( + !state.csharp_index_error.contains_key("frozen"), + "eviction must clear the cached C# error message along with the status" + ); +} diff --git a/src/symbols/csharp.rs b/src/symbols/csharp.rs index 52c04228..09cffab3 100644 --- a/src/symbols/csharp.rs +++ b/src/symbols/csharp.rs @@ -36,27 +36,26 @@ use std::collections::{HashMap, HashSet}; use std::io::{BufRead, BufReader}; use std::path::{Path, PathBuf}; use std::process::{Command, Stdio}; +use std::sync::Arc; use std::thread; use std::time::{SystemTime, UNIX_EPOCH}; use crate::lmdb_registry::TrackedEnv; use anyhow::{bail, Context, Result}; use heed::types::{Bytes, Str}; -use heed::{Database, EnvOpenOptions}; +use heed::Database; use serde::{Deserialize, Serialize}; use super::scip_parse; use super::{PrewarmSummary, RebuildScope, RebuildSummary, SymbolIndexer, SymbolReference}; -use crate::constants::{SCIP_LMDB_DEFAULT_MAP_SIZE_MB, SCIP_LMDB_MAP_SIZE_MB_ENV}; - // ── Constants ───────────────────────────────────────────────────── /// LMDB database name for the SCIP symbol table (definitions only after Opt 2). const SCIP_DB_NAME: &str = crate::constants::SCIP_SYMBOLS_DB_NAME; /// LMDB database name for the rebuild timestamp. -const SCIP_META_DB_NAME: &str = "scip_meta"; +const SCIP_META_DB_NAME: &str = crate::constants::SCIP_META_DB_NAME; /// LMDB database name for the position-to-symbols index. const SCIP_POSITION_DB_NAME: &str = crate::constants::SCIP_POSITION_DB_NAME; @@ -390,44 +389,15 @@ impl CSharpSymbolIndexer { None } - /// Open or create the SCIP LMDB environment for a given repo database path. + /// Open the shared SCIP LMDB environment for a given repo database path. /// - /// Pre-opens ALL named databases so they exist before first use. - /// LMDB requires named DBs to be created (or opened) in a write txn - /// before they can be read in later read txns within the same env session. - fn open_scip_env(&self, db_path: &Path) -> Result { - let scip_dir = db_path.join("scip"); - std::fs::create_dir_all(&scip_dir) - .with_context(|| format!("Failed to create SCIP directory: {}", scip_dir.display()))?; - - // SAFETY: same pattern as vectordb/store.rs — LMDB mmap contract. - // TrackedEnv additionally prevents double-open within the same process. - // - // map_size is virtual address space (not RSS). 512 MB default is safe on - // both Windows and POSIX; the OS only faults in pages that are written. - // Enterprise repos with thousands of symbols + Phase-3 ref_cache can - // exceed the old 64 MB limit, causing MDB_MAP_FULL on cache writes. - let map_size_mb = std::env::var(SCIP_LMDB_MAP_SIZE_MB_ENV) - .ok() - .and_then(|s| s.parse::().ok()) - .unwrap_or(SCIP_LMDB_DEFAULT_MAP_SIZE_MB); - let mut opts = EnvOpenOptions::new(); - opts.map_size(map_size_mb * 1024 * 1024).max_dbs(10); - // SAFETY: `NO_TLS` only changes reader-slot tracking. See `BASE_ENV_FLAGS`. - unsafe { opts.flags(crate::lmdb_registry::BASE_ENV_FLAGS) }; - let env = - unsafe { TrackedEnv::open(&opts, &scip_dir, &format!("SCIP({})", db_path.display()))? }; - - // Eagerly create / re-open all named databases. - let mut wtxn = env.write_txn()?; - env.create_database::(&mut wtxn, Some(SCIP_DB_NAME))?; - env.create_database::(&mut wtxn, Some(SCIP_META_DB_NAME))?; - env.create_database::(&mut wtxn, Some(SCIP_POSITION_DB_NAME))?; - env.create_database::(&mut wtxn, Some(SCIP_SIMPLE_NAMES_DB_NAME))?; - env.create_database::(&mut wtxn, Some(SCIP_REF_CACHE_DB_NAME))?; - wtxn.commit()?; - - Ok(env) + /// Delegates to [`crate::symbols::get_shared_scip_env`]: one environment + /// per `db_path/scip` for the whole process, shared across concurrent + /// queries, rebuilds and the TypeScript adapter, so overlapping users + /// serialise on LMDB's writer mutex instead of failing the double-open + /// guard. + fn open_scip_env(&self, db_path: &Path) -> Result> { + crate::symbols::get_shared_scip_env(db_path) } // ── Helper invocation ────────────────────────────────────────── @@ -674,9 +644,9 @@ impl CSharpSymbolIndexer { /// /// Inner implementation: fetch references for an EXACT (canonical) symbol key. /// - /// Opens its own LMDB environment so the caller's env handle (if any) is not - /// held concurrently with the internal write txn that caches lazy results. - /// This avoids the "two Env objects on the same path" footgun. + /// Uses the shared SCIP env ([`crate::symbols::get_shared_scip_env`]); the + /// internal write txn that caches lazy results serialises against other + /// writers on LMDB's single-writer mutex instead of erroring the loser. fn find_refs_for_canonical_key( &self, db_path: &Path, @@ -1531,9 +1501,9 @@ impl SymbolIndexer for CSharpSymbolIndexer { } fn find_references(&self, db_path: &Path, symbol: &str) -> Result> { - // Resolve to canonical key in a short-lived env scope, then drop it before - // entering find_refs_for_canonical_key (which opens its own env). - // This ensures no two Env handles are live on the same path simultaneously. + // Resolve to canonical key, then delegate. The env handle is the + // process-shared one; this scope just bounds how long we hold a + // reference — concurrent opens are impossible by construction. let canonical = { let env = self.open_scip_env(db_path)?; match self.resolve_canonical_key(&env, symbol)? { @@ -1573,7 +1543,7 @@ impl SymbolIndexer for CSharpSymbolIndexer { // Pick shortest (most specific) symbol defined at this position let chosen = candidate_keys.iter().min_by_key(|k| k.len()).cloned(); drop(rtxn); - drop(env); // must drop before find_refs_for_canonical_key opens its own env + drop(env); // release this reference before delegation; the shared env may stay alive match chosen { Some(k) => self.find_refs_for_canonical_key(db_path, &k), diff --git a/src/symbols/mod.rs b/src/symbols/mod.rs index a20f6d1d..f7cb6293 100644 --- a/src/symbols/mod.rs +++ b/src/symbols/mod.rs @@ -14,7 +14,7 @@ pub mod typescript; use std::path::{Path, PathBuf}; -use anyhow::Result; +use anyhow::{Context, Result}; use serde::{Deserialize, Serialize}; // ── Common types ────────────────────────────────────────────────── @@ -79,6 +79,72 @@ pub enum RebuildScope { }, } +// ── Shared SCIP environment ────────────────────────────────────── + +/// Open (or reuse) the process-wide shared SCIP LMDB environment for `db_path`. +/// +/// Both the C# and TypeScript adapters store symbol data in the same +/// `db_path/scip` directory, and LMDB allows exactly ONE open environment per +/// directory per process. Historically every operation opened its own +/// short-lived env, so two overlapping operations — e.g. a watcher-triggered +/// rebuild starting while a lazy `find-refs` call held its env for minutes — +/// tripped the double-open guard and one side failed outright +/// (`LMDB double-open prevented`, surfaced as a red `C#!` in the TUI). +/// Routing every open through this getter hands all concurrent users the SAME +/// environment: writers serialise on LMDB's single-writer mutex, readers never +/// block, and the double-open error class cannot occur. +pub(crate) fn get_shared_scip_env( + db_path: &Path, +) -> Result> { + let scip_dir = db_path.join("scip"); + std::fs::create_dir_all(&scip_dir) + .with_context(|| format!("Failed to create SCIP directory: {}", scip_dir.display()))?; + + crate::lmdb_registry::get_or_open_shared_env( + &scip_dir, + &format!("SCIP({})", db_path.display()), + |opts| { + // map_size is virtual address space (not RSS); the OS only faults + // in written pages. Read once per env lifetime. + let map_size_mb = std::env::var(crate::constants::SCIP_LMDB_MAP_SIZE_MB_ENV) + .ok() + .and_then(|s| s.parse::().ok()) + .unwrap_or(crate::constants::SCIP_LMDB_DEFAULT_MAP_SIZE_MB); + opts.map_size(map_size_mb * 1024 * 1024).max_dbs(10); + // SAFETY: `NO_TLS` only changes reader-slot tracking. See `BASE_ENV_FLAGS`. + unsafe { opts.flags(crate::lmdb_registry::BASE_ENV_FLAGS) }; + }, + |env| { + // Pre-create every named database (both languages') exactly once + // per env session: LMDB requires named DBs to exist before they + // can be opened in read txns. + let mut wtxn = env.write_txn()?; + env.create_database::( + &mut wtxn, + Some(crate::constants::SCIP_SYMBOLS_DB_NAME), + )?; + env.create_database::( + &mut wtxn, + Some(crate::constants::SCIP_META_DB_NAME), + )?; + env.create_database::( + &mut wtxn, + Some(crate::constants::SCIP_POSITION_DB_NAME), + )?; + env.create_database::( + &mut wtxn, + Some(crate::constants::SCIP_SIMPLE_NAMES_DB_NAME), + )?; + env.create_database::( + &mut wtxn, + Some(crate::constants::SCIP_REF_CACHE_DB_NAME), + )?; + wtxn.commit()?; + Ok(()) + }, + ) +} + /// Summary returned after a rebuild completes. #[derive(Debug, Clone)] #[allow(dead_code)] @@ -235,3 +301,21 @@ impl Default for SymbolIndexerRegistry { Self::new() } } + +#[cfg(test)] +mod tests { + use super::*; + + /// The scip getter must hand out ONE shared env per `db_path` — the exact + /// property that stops a rebuild and an in-flight lazy find-refs (or the + /// TypeScript adapter) from failing each other with the double-open error. + #[test] + fn shared_scip_env_is_shared_across_calls() { + let dir = tempfile::TempDir::new().unwrap(); + let db_path = dir.path().join("codesearch.db"); + + let env1 = get_shared_scip_env(&db_path).unwrap(); + let env2 = get_shared_scip_env(&db_path).unwrap(); + assert!(std::sync::Arc::ptr_eq(&env1, &env2)); + } +} diff --git a/src/symbols/typescript.rs b/src/symbols/typescript.rs index ca972b4a..f6dfef34 100644 --- a/src/symbols/typescript.rs +++ b/src/symbols/typescript.rs @@ -20,20 +20,20 @@ use std::path::{Path, PathBuf}; use std::process::{Command, Stdio}; +use std::sync::Arc; use std::time::{SystemTime, UNIX_EPOCH}; use crate::lmdb_registry::TrackedEnv; use anyhow::{Context, Result}; use heed::types::{Bytes, Str}; -use heed::{Database, EnvOpenOptions}; +use heed::Database; use serde::{Deserialize, Serialize}; use super::scip_proto; use super::{RebuildScope, RebuildSummary, SymbolIndexer, SymbolReference}; use crate::constants::{ - LANG_TYPESCRIPT, SCIP_LMDB_DEFAULT_MAP_SIZE_MB, SCIP_LMDB_MAP_SIZE_MB_ENV, - SCIP_POSITION_DB_NAME, SCIP_SIMPLE_NAMES_DB_NAME, SCIP_SYMBOLS_DB_NAME, + LANG_TYPESCRIPT, SCIP_POSITION_DB_NAME, SCIP_SIMPLE_NAMES_DB_NAME, SCIP_SYMBOLS_DB_NAME, SCIP_TYPESCRIPT_HELPER_ENV, SCIP_TYPESCRIPT_REBUILD_TIMESTAMP_KEY, }; @@ -45,7 +45,7 @@ const SCIP_DB_NAME: &str = SCIP_SYMBOLS_DB_NAME; /// LMDB database name for the rebuild timestamp / metadata table. /// Shares the physical table with the C# adapter, but keys are namespaced /// per-language (see `SCIP_TYPESCRIPT_REBUILD_TIMESTAMP_KEY`). -const SCIP_META_DB_NAME: &str = "scip_meta"; +const SCIP_META_DB_NAME: &str = crate::constants::SCIP_META_DB_NAME; /// LMDB database name for the position-to-symbols index. const SCIP_POS_DB_NAME: &str = SCIP_POSITION_DB_NAME; @@ -263,33 +263,14 @@ impl TypeScriptSymbolIndexer { } } - /// Open or create the SCIP LMDB environment for a given repo database path. - /// Shares the same on-disk tables as the C# adapter (`db_path/scip/`), - /// distinguished by namespaced keys/values where needed. - fn open_scip_env(&self, db_path: &Path) -> Result { - let scip_dir = db_path.join("scip"); - std::fs::create_dir_all(&scip_dir) - .with_context(|| format!("Failed to create SCIP directory: {}", scip_dir.display()))?; - - let map_size_mb = std::env::var(SCIP_LMDB_MAP_SIZE_MB_ENV) - .ok() - .and_then(|s| s.parse::().ok()) - .unwrap_or(SCIP_LMDB_DEFAULT_MAP_SIZE_MB); - let mut opts = EnvOpenOptions::new(); - opts.map_size(map_size_mb * 1024 * 1024).max_dbs(10); - // SAFETY: `NO_TLS` only changes reader-slot tracking. See `BASE_ENV_FLAGS`. - unsafe { opts.flags(crate::lmdb_registry::BASE_ENV_FLAGS) }; - let env = - unsafe { TrackedEnv::open(&opts, &scip_dir, &format!("SCIP({})", db_path.display()))? }; - - let mut wtxn = env.write_txn()?; - env.create_database::(&mut wtxn, Some(SCIP_DB_NAME))?; - env.create_database::(&mut wtxn, Some(SCIP_META_DB_NAME))?; - env.create_database::(&mut wtxn, Some(SCIP_POS_DB_NAME))?; - env.create_database::(&mut wtxn, Some(SCIP_NAMES_DB_NAME))?; - wtxn.commit()?; - - Ok(env) + /// Open the shared SCIP LMDB environment for a given repo database path. + /// + /// Delegates to [`crate::symbols::get_shared_scip_env`] — TS shares the + /// C# adapter's `db_path/scip/` directory (distinguished by namespaced + /// keys), so it must also share its environment: two adapters opening the + /// same directory concurrently would trip the double-open guard. + fn open_scip_env(&self, db_path: &Path) -> Result> { + crate::symbols::get_shared_scip_env(db_path) } /// Invoke `scip-typescript index` against `project_root`, writing the SCIP