Skip to content

fix(core): open LMDB envs WithoutTls to stop reader slot leak (#664)#665

Open
gustav-fff wants to merge 1 commit into
mainfrom
triage-bot/issue-664
Open

fix(core): open LMDB envs WithoutTls to stop reader slot leak (#664)#665
gustav-fff wants to merge 1 commit into
mainfrom
triage-bot/issue-664

Conversation

@gustav-fff

Copy link
Copy Markdown
Collaborator

Closes #664

Root cause

Default heed::EnvOpenOptions opens envs in WithTls mode. In this mode LMDB ties reader locktable slots to OS threads instead of MDB_txn objects, so any thread that ever calls read_txn() occupies a slot for the entire process lifetime — the slot is not released on commit() or drop().

fff opens read txns from many threads: rayon workers in BACKGROUND_THREAD_POOL, the background_watcher, the fff-lmdb-gc thread, git_status_worker, and the neovim main thread. maxreaders defaults to 126, so a handful of long-running nvim sessions burn through the entire reader table. New nvim sessions then hit MDB_READERS_FULL in open_database_safe() at crates/fff-core/src/dbs/lmdb.rs:210, which propagates out as Error::DbStartReadTxn and ultimately manifests as the SIGSEGV in the reporter's log:

ERROR ThreadId(02) fff_nvim::error: string_value="Failed to start read transaction for frecency database: MDB_READERS_FULL: Environment maxreaders limit reached"
=== CRASH SIGSEGV (fff) ===

clear_stale_readers() at env open only reclaims slots from dead processes, so live nvim instances happily hoard them.

Fix

Open the env with .read_txn_without_tls() (heed 0.22's WithoutTls marker). Reader slots are now tied to MDB_txn objects and released on commit() / drop(). Everything downstream (FrecencyTracker, QueryTracker, DbHealthChecker, LmdbStore) is retyped to Env<WithoutTls>; no runtime behavior changes for callers.

Steps to reproduce

Pre-fix, on main (1a8ef35):

git checkout main
cat > crates/fff-core/tests/repro_664.rs <<'RUST'
use fff_search::frecency::FrecencyTracker;
use std::sync::Arc;
#[test]
fn repro() {
    let tmp = tempfile::TempDir::new().unwrap();
    let t = Arc::new(FrecencyTracker::open(tmp.path()).unwrap());
    let handles: Vec<_> = (0..400).map(|i| {
        let t = Arc::clone(&t);
        std::thread::spawn(move || {
            let p = std::path::PathBuf::from(format!("/tmp/x/{i}"));
            t.access_count(&p).expect("should not fail");
        })
    }).collect();
    for h in handles { h.join().unwrap(); }
}
RUST
cargo test -p fff-search --test repro_664

Actual (pre-fix): fails ~thread 127 with MDB_READERS_FULL: Environment maxreaders limit reached.
Expected: all 400 threads complete successfully.

In the wild: 7+ long-running nvim sessions with fff.nvim loaded, then open picker in a new nvim — crashes with SIGSEGV.

How verified

  • cargo test -p fff-search --lib — 98 pass.
  • cargo test -p fff-search --test lmdb_reader_slot_leak — new regression test passes; reverting lmdb.rs fails the workspace to compile, confirming the trait plumbing is what enforces the flag.
  • cargo check --workspace — clean.

Automated triage via Gustav. Honk-Honk 🪿

Default heed EnvOpenOptions opens envs in WithTls mode, where LMDB pins
reader locktable slots to OS threads. Every thread that ever runs
read_txn() permanently occupies a slot for the process lifetime, even
after commit(). fff opens read txns from many threads (rayon workers,
background_watcher, fff-lmdb-gc, git_status_worker, neovim main), so a
handful of long-running nvim sessions exhaust the default 126-slot
table and new nvim processes crash with MDB_READERS_FULL.

Switch to read_txn_without_tls() — reader slots are now tied to MDB_txn
objects and released on commit()/drop().

Regression test spawns 400 short-lived reader threads against a fresh
FrecencyTracker; pre-fix it fails around thread 127 with
MDB_READERS_FULL, post-fix all 400 succeed.
@liu-qingyuan

Copy link
Copy Markdown

I tested this PR against a real MDB_READERS_FULL failure in the FFF integration used by pi-pretty, and the WithoutTls change fixes the issue in practice.

Environment:

  • FFF v0.10.0 via @ff-labs/fff-node
  • Apple Silicon macOS, 18 logical CPUs
  • 11 long-running Pi processes sharing one frecency database

Before the patch, the LMDB reader table had all 126 slots occupied across 11 live PIDs, with 10-12 slots per process. Every entry had txnid = -, so these were not active read transactions; they were TLS slots retained by long-lived FFF worker threads. A new process consistently failed with:

MDB_READERS_FULL: Environment maxreaders limit reached

I cherry-picked ff1de81 onto v0.10.0 and built the macOS arm64 C library using the release configuration (ci profile, zlob, macOS 13 target). Results:

  • 124 fff-search unit tests passed.
  • C scan/search/watch smoke tests passed.
  • Node API initialization and search passed.
  • The exported C symbol set was unchanged.
  • The patched process successfully initialized and scanned the real shared database while old processes still occupied 115/126 slots.
  • Most importantly, I kept the patched process alive after scanning and inspected both the frecency and history reader tables. There were zero reader entries for the patched PID. The old processes still showed their expected 10-12 retained entries each.

This directly confirms that read_txn_without_tls() prevents the persistent reader-slot accumulation in a real long-lived process.

One test note: the current 400 short-lived-thread regression test also passed for me on unpatched v0.10.0 on macOS, likely because the pthread TLS destructor clears each slot when those threads exit. A deterministic red/green test could keep every reader thread alive after access_count() using a ready/release channel; WithTls should then exhaust the table around reader 127, while WithoutTls should allow all completed transactions without retained slots.

This issue is reproducible in normal multi-process agent usage, and this PR resolved it here without changing the C/Node ABI. Please consider merging it soon. I am happy to test an updated branch as well.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

SIGSEGV crash: LMDB reader slot leak causes MDB_READERS_FULL after multiple nvim sessions

2 participants