Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion crates/fff-core/src/dbs/db_healthcheck.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ pub struct DbHealth {
}

pub trait DbHealthChecker {
fn get_env(&self) -> &heed::Env;
fn get_env(&self) -> &heed::Env<heed::WithoutTls>;
fn is_healthy(&self) -> bool;
/// Entries per database, each group has a static string label
fn count_entries(&self) -> Result<Vec<(&'static str, u64)>>;
Expand Down
12 changes: 6 additions & 6 deletions crates/fff-core/src/dbs/frecency.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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};

Expand All @@ -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<WithoutTls>,
db: Database<Bytes, SerdeBincode<VecDeque<u64>>>,
health: DbHealth,
}
Expand All @@ -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<WithoutTls> {
&self.env
}

Expand Down Expand Up @@ -77,15 +77,15 @@ 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<WithoutTls> {
&self.env
}

fn health(&self) -> &DbHealth {
&self.health
}

fn purge_stale_data(env: &Env) -> Result<()> {
fn purge_stale_data(env: &Env<WithoutTls>) -> Result<()> {
let (deleted, pruned) = Self::purge_stale_entries(env)?;
if deleted > 0 || pruned > 0 {
tracing::info!(deleted, pruned, "Frecency GC purged entries");
Expand Down Expand Up @@ -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<WithoutTls>) -> Result<(usize, usize)> {
let now = SystemTime::now()
.duration_since(UNIX_EPOCH)
.unwrap()
Expand Down
20 changes: 14 additions & 6 deletions crates/fff-core/src/dbs/lmdb.rs
Original file line number Diff line number Diff line change
@@ -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;
Expand Down Expand Up @@ -128,31 +128,36 @@ 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<WithoutTls>;
/// 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<WithoutTls>) -> Result<()> {
Ok(())
}

/// Open the LMDB env. Returns env + a `DbHealth` starting in Pending;
/// 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<WithoutTls>, DbHealth)> {
Self::erase_if_oversized(db_path);
fs::create_dir_all(db_path).map_err(Error::CreateDir)?;
let db = Self::LABEL;

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);
Expand Down Expand Up @@ -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<KC, DC>(env: &Env, name: Option<&str>) -> Result<Database<KC, DC>>
fn open_database_safe<KC, DC>(
env: &Env<WithoutTls>,
name: Option<&str>,
) -> Result<Database<KC, DC>>
where
KC: 'static,
DC: 'static,
Expand Down
10 changes: 5 additions & 5 deletions crates/fff-core/src/dbs/query_tracker.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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};
Expand All @@ -27,7 +27,7 @@ struct HistoryEntry {

#[derive(Debug)]
pub struct QueryTracker {
env: Env,
env: Env<WithoutTls>,
// Database for (project_path, query) -> QueryMatchEntry mappings
query_file_db: Database<Bytes, SerdeBincode<QueryMatchEntry>>,
// Database for project_path -> VecDeque<HistoryEntry> mappings (file picker)
Expand All @@ -38,7 +38,7 @@ pub struct QueryTracker {
}

impl DbHealthChecker for QueryTracker {
fn get_env(&self) -> &Env {
fn get_env(&self) -> &Env<WithoutTls> {
&self.env
}

Expand Down Expand Up @@ -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<WithoutTls> {
&self.env
}

Expand Down Expand Up @@ -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<Bytes, SerdeBincode<VecDeque<HistoryEntry>>>,
env: &Env,
env: &Env<WithoutTls>,
project_key: &[u8; 32],
offset: usize,
) -> Result<Option<String>, Error> {
Expand Down
44 changes: 44 additions & 0 deletions crates/fff-core/tests/lmdb_reader_slot_leak.rs
Original file line number Diff line number Diff line change
@@ -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();
}
}
Loading