diff --git a/crates/fff-core/src/dbs/db_healthcheck.rs b/crates/fff-core/src/dbs/db_healthcheck.rs index 77d83499..6ed8fd53 100644 --- a/crates/fff-core/src/dbs/db_healthcheck.rs +++ b/crates/fff-core/src/dbs/db_healthcheck.rs @@ -14,7 +14,7 @@ pub struct DbHealth { } pub trait DbHealthChecker { - fn get_env(&self) -> &heed::Env; + fn get_env(&self) -> &heed::Env; fn is_healthy(&self) -> bool; /// Entries per database, each group has a static string label fn count_entries(&self) -> Result>; diff --git a/crates/fff-core/src/dbs/frecency.rs b/crates/fff-core/src/dbs/frecency.rs index 7413396a..3d41fcdd 100644 --- a/crates/fff-core/src/dbs/frecency.rs +++ b/crates/fff-core/src/dbs/frecency.rs @@ -4,7 +4,7 @@ use crate::error::{Error, Result}; use crate::file_picker::FFFMode; use crate::git::is_modified_status; use heed::types::{Bytes, SerdeBincode}; -use heed::{Database, Env}; +use heed::{Database, Env, WithoutTls}; use std::time::{SystemTime, UNIX_EPOCH}; use std::{collections::VecDeque, path::Path}; @@ -19,7 +19,7 @@ const AI_MAX_HISTORY_DAYS: f64 = 7.0; // Only consider accesses within 7 days #[derive(Debug)] pub struct FrecencyTracker { - env: Env, + env: Env, db: Database>>, health: DbHealth, } @@ -42,7 +42,7 @@ const AI_MODIFICATION_THRESHOLDS: [(i64, u64); 5] = [ ]; impl DbHealthChecker for FrecencyTracker { - fn get_env(&self) -> &heed::Env { + fn get_env(&self) -> &heed::Env { &self.env } @@ -77,7 +77,7 @@ impl LmdbStore for FrecencyTracker { // MAP_SIZE so we don't hit MDB_MAP_FULL before the open-time erase fires. const SIZE_CAP_BYTES: u64 = 12 * 1024 * 1024; - fn env(&self) -> &Env { + fn env(&self) -> &Env { &self.env } @@ -85,7 +85,7 @@ impl LmdbStore for FrecencyTracker { &self.health } - fn purge_stale_data(env: &Env) -> Result<()> { + fn purge_stale_data(env: &Env) -> Result<()> { let (deleted, pruned) = Self::purge_stale_entries(env)?; if deleted > 0 || pruned > 0 { tracing::info!(deleted, pruned, "Frecency GC purged entries"); @@ -121,7 +121,7 @@ impl FrecencyTracker { /// Removes entries where all timestamps are older than MAX_HISTORY_DAYS, /// and prunes stale timestamps from entries that still have recent ones. /// Returns (deleted_count, pruned_count). - fn purge_stale_entries(env: &Env) -> Result<(usize, usize)> { + fn purge_stale_entries(env: &Env) -> Result<(usize, usize)> { let now = SystemTime::now() .duration_since(UNIX_EPOCH) .unwrap() diff --git a/crates/fff-core/src/dbs/lmdb.rs b/crates/fff-core/src/dbs/lmdb.rs index 9eba707c..37aef0e7 100644 --- a/crates/fff-core/src/dbs/lmdb.rs +++ b/crates/fff-core/src/dbs/lmdb.rs @@ -1,4 +1,4 @@ -use heed::{Database, Env, EnvOpenOptions}; +use heed::{Database, Env, EnvOpenOptions, WithoutTls}; use std::fs; use std::path::Path; use std::sync::Arc; @@ -128,14 +128,14 @@ pub(crate) trait LmdbStore: Sized + Send + Sync + 'static { const SIZE_CAP_BYTES: u64; /// Borrow the env in the read lock - fn env(&self) -> &Env; + fn env(&self) -> &Env; /// Borrow the health flag from the tracker. fn health(&self) -> &DbHealth; /// Override to purge stale rows, compact, etc. Default no-op. Runs on /// the GC thread while a read lock is held against the shared handle, /// so destroy / re-init naturally wait for it. - fn purge_stale_data(_env: &Env) -> Result<()> { + fn purge_stale_data(_env: &Env) -> Result<()> { Ok(()) } @@ -143,7 +143,7 @@ pub(crate) trait LmdbStore: Sized + Send + Sync + 'static { /// the GC thread spawned by `spawn_gc` flips it to Healthy. Write /// paths flip it to Degraded on MDB_MAP_FULL. #[tracing::instrument] - fn open_env(db_path: &Path) -> Result<(Env, DbHealth)> { + fn open_env(db_path: &Path) -> Result<(Env, DbHealth)> { Self::erase_if_oversized(db_path); fs::create_dir_all(db_path).map_err(Error::CreateDir)?; let db = Self::LABEL; @@ -151,8 +151,13 @@ pub(crate) trait LmdbStore: Sized + Send + Sync + 'static { const MAX_ATTEMPTS: u32 = 8; let mut attempt = 0u32; let env = loop { + // read_txn_without_tls: reader slots are tied to `MDB_txn` objects + // instead of OS threads. Without this, each thread that ever opens + // a read txn holds a slot for the lifetime of the process — rayon + // workers, watcher, GC and main thread quickly exhaust maxreaders + // and new nvim sessions crash with MDB_READERS_FULL (issue #664). let result = unsafe { - let mut opts = EnvOpenOptions::new(); + let mut opts = EnvOpenOptions::new().read_txn_without_tls(); opts.map_size(Self::MAP_SIZE); if Self::MAX_DBS > 0 { opts.max_dbs(Self::MAX_DBS); @@ -200,7 +205,10 @@ pub(crate) trait LmdbStore: Sized + Send + Sync + 'static { /// Open or create a database without blocking on the LMDB writer mutex /// when the database already exists. - fn open_database_safe(env: &Env, name: Option<&str>) -> Result> + fn open_database_safe( + env: &Env, + name: Option<&str>, + ) -> Result> where KC: 'static, DC: 'static, diff --git a/crates/fff-core/src/dbs/query_tracker.rs b/crates/fff-core/src/dbs/query_tracker.rs index 40d01178..03ef4a33 100644 --- a/crates/fff-core/src/dbs/query_tracker.rs +++ b/crates/fff-core/src/dbs/query_tracker.rs @@ -2,7 +2,7 @@ use super::db_healthcheck::DbHealthChecker; use super::lmdb::{DbHealth, LmdbStore, is_map_full}; use crate::error::Error; use heed::types::{Bytes, SerdeBincode}; -use heed::{Database, Env}; +use heed::{Database, Env, WithoutTls}; use serde::{Deserialize, Serialize}; use std::collections::VecDeque; use std::path::{Path, PathBuf}; @@ -27,7 +27,7 @@ struct HistoryEntry { #[derive(Debug)] pub struct QueryTracker { - env: Env, + env: Env, // Database for (project_path, query) -> QueryMatchEntry mappings query_file_db: Database>, // Database for project_path -> VecDeque mappings (file picker) @@ -38,7 +38,7 @@ pub struct QueryTracker { } impl DbHealthChecker for QueryTracker { - fn get_env(&self) -> &Env { + fn get_env(&self) -> &Env { &self.env } @@ -92,7 +92,7 @@ impl LmdbStore for QueryTracker { const MAX_DBS: u32 = 16; const SIZE_CAP_BYTES: u64 = 8 * 1024 * 1024; - fn env(&self) -> &Env { + fn env(&self) -> &Env { &self.env } @@ -197,7 +197,7 @@ impl QueryTracker { /// offset=0 returns most recent, offset=1 returns 2nd most recent, etc. fn read_history_at_offset( db: &Database>>, - env: &Env, + env: &Env, project_key: &[u8; 32], offset: usize, ) -> Result, Error> { diff --git a/crates/fff-core/tests/lmdb_reader_slot_leak.rs b/crates/fff-core/tests/lmdb_reader_slot_leak.rs new file mode 100644 index 00000000..8fc7746a --- /dev/null +++ b/crates/fff-core/tests/lmdb_reader_slot_leak.rs @@ -0,0 +1,44 @@ +//! Regression test for #664. +//! +//! Default `heed::EnvOpenOptions` opens the env in `WithTls` mode, where LMDB +//! ties reader locktable slots to OS threads instead of `MDB_txn` objects. Every +//! thread that ever calls `read_txn()` occupies a reader slot for the lifetime +//! of the process, even after the txn commits. fff hits this with rayon workers, +//! the background watcher, the LMDB GC thread and the neovim main thread — a +//! handful of long-running nvim sessions exhaust the default 126-slot table and +//! new nvim processes crash with `MDB_READERS_FULL`. +//! +//! The fix opens the env with `read_txn_without_tls()`. This test spawns many +//! more short-lived threads than `maxreaders` and confirms none of them fail; +//! without the fix it fails around thread 127. + +use fff_search::frecency::FrecencyTracker; +use std::sync::Arc; +use std::thread; + +#[test] +fn read_txns_from_many_threads_do_not_exhaust_readers() { + let tmp = tempfile::TempDir::new().unwrap(); + let tracker = Arc::new(FrecencyTracker::open(tmp.path()).unwrap()); + + // Well above LMDB's default maxreaders (126). With WithTls each of these + // would permanently pin a slot; ~127th thread would return MDB_READERS_FULL. + const N: usize = 400; + let handles: Vec<_> = (0..N) + .map(|i| { + let t = Arc::clone(&tracker); + thread::Builder::new() + .name(format!("reader-{i}")) + .spawn(move || { + let path = std::path::PathBuf::from(format!("/tmp/frecency-slot-leak/{i}")); + t.access_count(&path) + .expect("read txn should not fail with MDB_READERS_FULL") + }) + .unwrap() + }) + .collect(); + + for h in handles { + h.join().unwrap(); + } +}