fix: Race on immediate file delete after init

closes #515

Makefile

@@ -14,7 +14,7 @@ SHELL := bash
 # string rather than the literal `-o` / `pipefail` tokens.
 .SHELLFLAGS := -o pipefail -ec
 
-.PHONY: build build-c-lib install uninstall test test-rust test-lua test-lua-snap test-version test-bun test-node prepare-bun prepare-node set-npm-version header test-stress test-stress-seeded test-stress-random test-stress-repos
+.PHONY: build build-c-lib install uninstall test test-rust test-lua test-lua-snap test-version test-bun test-node prepare-bun prepare-node set-npm-version header test-stress test-stress-seeded test-stress-random test-stress-repos test-node-stress
 
 all: format test lint
 
@@ -125,7 +125,14 @@ test-bun: prepare-bun
 test-node: prepare-node
 	cd packages/fff-node && npm run build && node test/e2e.mjs
 
-test: test-rust test-lua test-lua-snap test-version test-bun test-node
+# Bug pinning stress test script over fff-node for issue #515
+# Just keep it untouched because it's good enough + some stress for SDK
+FFF_STRESS_ITERS ?= 50
+test-node-stress: prepare-node
+	cd packages/fff-node && npm run build && \
+		FFF_STRESS_ITERS=$(FFF_STRESS_ITERS) node test/stress-515.mjs
+
+test: test-rust test-lua test-lua-snap test-version test-bun test-node test-node-stress
 
 test-stress-seeded:
 	FFF_STRESS_SEED="$${FFF_STRESS_SEED:-$(FFF_STRESS_DEFAULT_SEED)}" \

crates/fff-core/src/bigram_filter.rs

@@ -610,31 +610,19 @@ thread_local! {
         std::cell::RefCell::new(vec![0u8; MAX_INDEXABLE_FILE_SIZE].into_boxed_slice());
 }
 
-/// reads a chunk for bigram either from new warmed up cache or from the file directly
+/// Reads bigram chunk, we *SHOULD NOT* use mmap cache here because bigram is built off-lock
+/// if the watcher thread tries to invalidate mmap during the borrow from it - UAB or segfaut
+///
+/// mmap should only be used by the locked version of grep which absolutely minimizes any riscs
 #[inline]
-#[allow(clippy::too_many_arguments)]
 fn read_bigram_chunk<'a>(
-    file: &'a crate::types::FileItem,
+    file: &crate::types::FileItem,
     base_fd: libc::c_int,
     base_path: &std::path::Path,
     arena: crate::simd_path::ArenaPtr,
-    budget: &crate::types::ContentCacheBudget,
-    warmup: bool,
     buf: &'a mut [u8],
     path_buf: &mut [u8; crate::simd_path::PATH_BUF_SIZE],
 ) -> Option<&'a [u8]> {
-    if warmup
-        && !file.is_likely_hot()
-        && let Some(cached) = file.get_cached_content(arena, base_path, budget)
-    {
-        if crate::file_picker::detect_binary_content(cached) {
-            file.set_binary(true);
-            return None;
-        }
-
-        return Some(&cached[..cached.len().min(MAX_INDEXABLE_FILE_SIZE)]);
-    }
-
     let want = (file.size as usize).min(MAX_INDEXABLE_FILE_SIZE);
     let filled = file.read_trimmed_into_buf(base_fd, base_path, arena, path_buf, &mut buf[..want]);
     if filled == 0 {
@@ -652,10 +640,8 @@ fn read_bigram_chunk<'a>(
 #[tracing::instrument(skip_all, name = "Building Bigram Index", level = tracing::Level::DEBUG)]
 pub(crate) fn build_bigram_index(
     files: &[crate::types::FileItem],
-    budget: &crate::types::ContentCacheBudget,
     base_path: &std::path::Path,
     arena: crate::simd_path::ArenaPtr,
-    warmup: bool,
 ) -> BigramFilter {
     let builder = BigramIndexBuilder::new(files.len());
     let skip_builder = BigramIndexBuilder::new(files.len());
@@ -665,12 +651,11 @@ pub(crate) fn build_bigram_index(
     #[cfg(not(unix))]
     let base_fd: i32 = -1;
 
-    // Single unified pass: every file is bigram-indexed, and (when `warmup`)
-    // the content cache is opportunistically filled. We SKIP caching files
-    // that are likely already hot in the OS page cache (recent frecency hits
-    // or dirty-per-git) so our limited cache budget goes to the cold tail
-    // that actually benefits from a pinned mmap. Natural traversal order,
-    // no pre-sort, no separate warmup pass.
+    // Always reads each file into the thread-local READ_BUF — never aliases the
+    // persistent mmap cache. See `read_bigram_chunk` for the rationale: this
+    // pass runs detached on the background pool without holding the picker
+    // read lock, so a watcher event mutating a `FileItem` would race any
+    // borrow we took from a cached `Mmap`.
     crate::file_picker::BACKGROUND_THREAD_POOL.install(|| {
         files
             .par_chunks(BIGRAM_CHUNK_FILES)
@@ -693,8 +678,6 @@ pub(crate) fn build_bigram_index(
                             base_fd,
                             base_path,
                             arena,
-                            budget,
-                            warmup,
                             &mut buf[..],
                             &mut path_buf,
                         ) {

crates/fff-core/src/file_picker.rs

@@ -1903,6 +1903,7 @@ impl FileSync {
 
 /// Pre-populate mmap caches for cold tail files so the first grep search
 /// doesn't pay the mmap creation + page fault cost.
+#[allow(dead_code)]
 #[tracing::instrument(skip(files), name = "warmup_mmaps", level = Level::DEBUG)]
 pub(crate) fn warmup_mmaps(
     files: &[FileItem],

crates/fff-core/src/scan.rs

@@ -9,7 +9,7 @@ use crate::FileSync;
 use crate::background_watcher::BackgroundWatcher;
 use crate::bigram_filter::build_bigram_index;
 use crate::error::Error;
-use crate::file_picker::{BACKGROUND_THREAD_POOL, FFFMode, warmup_mmaps};
+use crate::file_picker::{BACKGROUND_THREAD_POOL, FFFMode};
 use crate::git::GitStatusCache;
 use crate::shared::{SharedFilePicker, SharedFrecency};
 use crate::simd_path::ArenaPtr;
@@ -277,6 +277,11 @@ impl ScanJob {
         }
     }
 
+    /// THIS IS VERY VERY IMPORTANT THAT ANYTHING INSIDE THIS FUNCTION TO NOT READ ANYTHING CLEARABLE OUTSIDE
+    /// this is a very silly off lock implementation that actually matters, and that's why it is crafted
+    /// to never read anything from the picker, it can only WRITE information using single instructions
+    ///
+    /// Things that are safe and immutable - file list, indexes of files, paths, and signals.
     #[tracing::instrument(skip_all, fields(warmup = ?config.warmup, indexing = ?config.content_indexing))]
     fn run_post_scan(
         shared_picker: &SharedFilePicker,
@@ -289,33 +294,28 @@ impl ScanJob {
             .as_ref()
             .map(|s| s.as_arena_ptr())
             .unwrap_or(ArenaPtr::null());
-        let budget: &ContentCacheBudget = &unsafe_snapshot.budget;
+        let _budget: &ContentCacheBudget = &unsafe_snapshot.budget;
         let files: &[crate::types::FileItem] = &unsafe_snapshot.files[..unsafe_snapshot.base_count];
 
         if signals.cancelled.load(Ordering::Acquire) {
             return;
         }
 
-        // unified bigram and warmup_mmaps in one go, it's important to reuse open files as much as possible
         if config.content_indexing {
             let indexable_files = &files[..unsafe_snapshot.indexable_count.min(files.len())];
-            let index = build_bigram_index(
-                indexable_files,
-                budget,
-                &unsafe_snapshot.base_path,
-                arena,
-                config.warmup, // can be optionally skipped
-            );
+            let index = build_bigram_index(indexable_files, &unsafe_snapshot.base_path, arena);
 
             if let Ok(mut guard) = shared_picker.write()
                 && let Some(picker) = guard.as_mut()
             {
                 picker.set_bigram_index(index);
             }
-        } else if config.warmup {
-            // Warmup-only: no bigram indexing, just fill the mmap cache.
-            warmup_mmaps(files, budget, &unsafe_snapshot.base_path, arena);
         }
+
+        // Skipped as potentially unsafe - figure this out later
+        // if config.warmup && !signals.cancelled.load(Ordering::Acquire) {
+        //     warmup_mmaps(files, budget, &unsafe_snapshot.base_path, arena);
+        // }
     }
 }
 

crates/fff-core/src/types.rs

@@ -214,6 +214,7 @@ pub struct FileItem {
     pub(crate) path: crate::simd_path::ChunkedString,
     pub(crate) parent_dir_index: u32,
     flags: AtomicU8,
+    /// Lazy mmap cache. Only populated by the actual file read, controlled by the budget.
     #[cfg(not(target_os = "windows"))]
     content: OnceLock<memmap2::Mmap>,
 }
@@ -381,6 +382,7 @@ impl FileItem {
         self.access_frecency_score as i32 + self.modification_frecency_score as i32
     }
 
+    #[allow(dead_code)]
     #[inline]
     pub(crate) fn is_likely_hot(&self) -> bool {
         self.access_frecency_score > 0 || self.git_status.is_some()
@@ -593,6 +595,15 @@ impl FileItem {
         None
     }
 
+    /// Returns a reference to a cached mmap of the file's contents.
+    ///
+    /// SAFETY-CRITICAL: callers must hold the picker read lock for as long as
+    /// the returned slice is in use. The watcher mutates `FileItem` (including
+    /// `invalidate_mmap`) under the picker write lock, so the read lock is
+    /// what prevents UAF (`OnceLock` reset → `munmap`) and SIGBUS (in-place
+    /// truncate → access past new EOF). Detached background tasks (e.g. the
+    /// bigram builder running on `BACKGROUND_THREAD_POOL`) MUST NOT call this
+    /// — use `read_trimmed_into_buf` instead.
     #[cfg(not(target_os = "windows"))]
     pub(crate) fn get_cached_content(
         &self,
@@ -648,7 +659,9 @@ impl FileItem {
         base_path: &Path,
         budget: &ContentCacheBudget,
     ) -> Option<&'a [u8]> {
-        // Fast path: persistent cache hit (zero-copy).
+        // Fast path: persistent cache hit (zero-copy). Safe here because grep
+        // callers hold the picker read lock for the lifetime of the returned
+        // slice — see [`Self::get_cached_content`] safety note.
         if let Some(cached) = self.get_cached_content(arena, base_path, budget) {
             return Some(cached);
         }

packages/fff-node/test/stress-515.mjs

@@ -0,0 +1,122 @@
+/**
+ * Stress reproducer for issue #515.
+ *
+ * Repeatedly creates a FileFinder, waits for scan, runs mixed file / dir /
+ * grep operations with periodic scanFiles() and refreshGitStatus() calls,
+ * destroys it, then repeats across two repos.
+ *
+ * Usage:
+ *   node test/stress-515.mjs [iterations] [repoA] [repoB]
+ */
+
+import { existsSync } from "node:fs";
+import { dirname, resolve } from "node:path";
+import process from "node:process";
+import { fileURLToPath } from "node:url";
+import { FileFinder } from "../dist/src/index.js";
+
+const __dirname = dirname(fileURLToPath(import.meta.url));
+const REPO_ROOT = resolve(__dirname, "..", "..", "..");
+
+const args = process.argv.slice(2);
+const ITERS = Number(args[0] || process.env.FFF_STRESS_ITERS || 50);
+const REPO_A = resolve(args[1] || process.env.FFF_STRESS_REPO_A || REPO_ROOT);
+const REPO_B_CANDIDATE = args[2] || process.env.FFF_STRESS_REPO_B || resolve(REPO_ROOT, "big-repo");
+const REPO_B = existsSync(REPO_B_CANDIDATE) ? resolve(REPO_B_CANDIDATE) : REPO_A;
+
+const REPOS = REPO_A === REPO_B ? [REPO_A] : [REPO_A, REPO_B];
+
+const SEARCH_QUERIES = [
+  "main",
+  "lib",
+  "fn",
+  "TODO",
+  "config",
+  "README",
+  "Cargo",
+  "test",
+  "src/",
+  "init",
+  "pub",
+  "use",
+];
+
+const GREP_QUERIES = [
+  "fn ",
+  "pub fn",
+  "TODO",
+  "FIXME",
+  "use std",
+  "impl ",
+  "struct ",
+  "let mut",
+  "return",
+  "match ",
+];
+
+function pick(arr) {
+  return arr[Math.floor(Math.random() * arr.length)];
+}
+
+async function runIteration(iter) {
+  const base = REPOS[iter % REPOS.length];
+  process.stdout.write(`[iter ${iter}] base=${base} `);
+
+  const created = FileFinder.create({
+    basePath: base,
+    logFilePath: `/tmp/fff-515-${iter}.log`,
+    logLevel: "debug",
+  });
+  if (!created.ok) {
+    console.error(`create failed: ${created.error}`);
+    return false;
+  }
+  const finder = created.value;
+
+  const wait = await finder.waitForScan(60_000);
+  if (!wait.ok || !wait.value) {
+    console.error(`waitForScan failed: ${JSON.stringify(wait)}`);
+    finder.destroy();
+    return false;
+  }
+
+  process.stdout.write("scan-done ");
+
+  const opCount = 30 + Math.floor(Math.random() * 30);
+  for (let i = 0; i < opCount; i++) {
+    const r = Math.random();
+    if (r < 0.35) {
+      finder.fileSearch(pick(SEARCH_QUERIES), { pageSize: 20 });
+    } else if (r < 0.55) {
+      finder.directorySearch(pick(SEARCH_QUERIES), { pageSize: 20 });
+    } else if (r < 0.85) {
+      finder.grep(pick(GREP_QUERIES), { mode: "plain", pageSize: 20 });
+    } else if (r < 0.93) {
+      finder.scanFiles();
+    } else {
+      finder.refreshGitStatus();
+    }
+  }
+
+  process.stdout.write("ops-done ");
+
+  finder.destroy();
+  process.stdout.write("destroyed\n");
+  return true;
+}
+
+(async () => {
+  console.log(`Running ${ITERS} iterations across:`);
+  console.log(`  A: ${REPO_A}`);
+  console.log(`  B: ${REPO_B}`);
+
+  for (let i = 0; i < ITERS; i++) {
+    const ok = await runIteration(i);
+    if (!ok) {
+      console.error(`Aborting at iteration ${i}`);
+      process.exit(1);
+    }
+  }
+
+  console.log("Completed without crash.");
+})();