Skip to content

Commit 0e73f19

Browse files
committed
fix: Race when openning many sequenital dirs
closes #269 closes #256
1 parent 10a27f9 commit 0e73f19

6 files changed

Lines changed: 76 additions & 35 deletions

File tree

README.md

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -282,8 +282,7 @@ require('fff').setup({
282282
#### Available Methods
283283

284284
```lua
285-
require('fff').find_files() -- Find files in current directory
286-
require('fff').find_in_git_root() -- Find files in the current git repository
285+
require('fff').find_files() -- Find files in current repositro
287286
require('fff').scan_files() -- Trigger rescan of files in the current directory
288287
require('fff').refresh_git_status() -- Refresh git status for the active file lock
289288
require('fff').find_files_in_dir(path) -- Find files in a specific directory

crates/fff-core/src/file_picker.rs

Lines changed: 32 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -186,6 +186,7 @@ pub struct FilePicker {
186186
scanned_files_count: Arc<AtomicUsize>,
187187
background_watcher: Option<BackgroundWatcher>,
188188
warmup_mmap_cache: bool,
189+
cancelled: Arc<AtomicBool>,
189190
}
190191

191192
impl std::fmt::Debug for FilePicker {
@@ -250,6 +251,7 @@ impl FilePicker {
250251
// rather than a stale `false` (the thread hasn't started yet).
251252
let scan_signal = Arc::new(AtomicBool::new(true));
252253
let synced_files_count = Arc::new(AtomicUsize::new(0));
254+
let cancelled = Arc::new(AtomicBool::new(false));
253255

254256
let picker = FilePicker {
255257
base_path: path.clone(),
@@ -258,6 +260,7 @@ impl FilePicker {
258260
scanned_files_count: Arc::clone(&synced_files_count),
259261
background_watcher: None,
260262
warmup_mmap_cache,
263+
cancelled: Arc::clone(&cancelled),
261264
};
262265

263266
// Place the picker into the shared handle before spawning the
@@ -274,6 +277,7 @@ impl FilePicker {
274277
warmup_mmap_cache,
275278
shared_picker,
276279
shared_frecency,
280+
cancelled,
277281
);
278282

279283
Ok(())
@@ -578,6 +582,11 @@ impl FilePicker {
578582
.retain_files(|file| !file.path.starts_with(dir_path))
579583
}
580584

585+
/// We use this to prevent any substantial background threads from acquiring the locks
586+
pub fn cancel(&self) {
587+
self.cancelled.store(true, Ordering::Release);
588+
}
589+
581590
pub fn stop_background_monitor(&mut self) {
582591
if let Some(watcher) = self.background_watcher.take() {
583592
watcher.stop();
@@ -644,6 +653,7 @@ fn spawn_scan_and_watcher(
644653
warmup_mmap_cache: bool,
645654
shared_picker: SharedPicker,
646655
shared_frecency: SharedFrecency,
656+
cancelled: Arc<AtomicBool>,
647657
) {
648658
std::thread::spawn(move || {
649659
// scan_signal is already `true` (set by the caller before spawning)
@@ -653,6 +663,12 @@ fn spawn_scan_and_watcher(
653663
let mut git_workdir = None;
654664
match scan_filesystem(&base_path, &synced_files_count, &shared_frecency) {
655665
Ok(sync) => {
666+
if cancelled.load(Ordering::Acquire) {
667+
info!("Scan completed but picker was replaced, discarding results");
668+
scan_signal.store(false, Ordering::Relaxed);
669+
return;
670+
}
671+
656672
info!(
657673
"Initial filesystem scan completed: found {} files",
658674
sync.files.len()
@@ -672,13 +688,8 @@ fn spawn_scan_and_watcher(
672688
}
673689

674690
// OPTIMIZATION: Warmup mmap cache in background to avoid blocking first grep.
675-
// The aggressive parallel warmup was causing cache thrashing and delaying
676-
// initial searches. Now it runs async and doesn't block.
677-
//
678-
// We warmup under a read lock on the picker's actual files so that
679-
// the OnceLock<Mmap> instances are populated in-place — no clone needed.
680-
// Read locks allow concurrent readers so this doesn't block searches.
681691
if warmup_mmap_cache
692+
&& !cancelled.load(Ordering::Acquire)
682693
&& let Ok(guard) = shared_picker.read()
683694
&& let Some(ref picker) = *guard
684695
{
@@ -691,6 +702,12 @@ fn spawn_scan_and_watcher(
691702
}
692703
scan_signal.store(false, Ordering::Relaxed);
693704

705+
// Don't create a watcher if this picker instance was already replaced
706+
if cancelled.load(Ordering::Acquire) {
707+
info!("Picker was replaced, skipping background watcher creation");
708+
return;
709+
}
710+
694711
match BackgroundWatcher::new(
695712
base_path,
696713
git_workdir,
@@ -700,6 +717,15 @@ fn spawn_scan_and_watcher(
700717
Ok(watcher) => {
701718
info!("Background file watcher initialized successfully");
702719

720+
// Final cancellation check: if the picker was replaced between
721+
// watcher creation and this write, drop the watcher instead of
722+
// storing it in the wrong picker.
723+
if cancelled.load(Ordering::Acquire) {
724+
info!("Picker was replaced, dropping orphaned watcher");
725+
drop(watcher);
726+
return;
727+
}
728+
703729
let write_result = shared_picker.write().ok().map(|mut guard| {
704730
if let Some(ref mut picker) = *guard {
705731
picker.background_watcher = Some(watcher);

crates/fff-nvim/src/lib.rs

Lines changed: 32 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -100,17 +100,24 @@ pub fn init_file_picker(_: &Lua, base_path: String) -> LuaResult<bool> {
100100
}
101101

102102
fn reinit_file_picker_internal(path: &Path) -> Result<(), Error> {
103-
// Stop existing picker
103+
// Cancel and stop the old picker under a single write lock to avoid
104+
// a window where FILE_PICKER is None (which causes FilePickerMissing
105+
// errors if the UI is searching concurrently).
104106
{
105107
let mut guard = FILE_PICKER
106108
.write()
107109
.with_lock_error(Error::AcquireItemLock)?;
108-
if let Some(mut picker) = guard.take() {
110+
if let Some(ref mut picker) = *guard {
111+
// Signal cancellation BEFORE stopping — this tells any orphaned
112+
// scan threads from this picker to discard their results.
113+
picker.cancel();
109114
picker.stop_background_monitor();
110115
}
116+
// Don't take() here — leave the old picker in place so searches
117+
// still work until new_with_shared_state replaces it atomically.
111118
}
112119

113-
// Create new picker backed by the same shared state
120+
// Create new picker — this atomically replaces the old one via write lock
114121
FilePicker::new_with_shared_state(
115122
path.to_string_lossy().to_string(),
116123
false,
@@ -134,6 +141,12 @@ pub fn restart_index_in_path(_: &Lua, new_path: String) -> LuaResult<()> {
134141
LuaError::RuntimeError(format!("Failed to canonicalize path '{}': {}", new_path, e))
135142
})?;
136143

144+
if let Ok(Some(picker)) = FILE_PICKER.read().as_deref() {
145+
if picker.base_path() == canonical_path {
146+
return Ok(()); // same dir
147+
}
148+
}
149+
137150
// Spawn a background thread to avoid blocking Lua/UI thread
138151
std::thread::spawn(move || {
139152
if let Err(e) = reinit_file_picker_internal(&canonical_path) {
@@ -564,21 +577,28 @@ pub fn get_historical_grep_query(_: &Lua, offset: usize) -> LuaResult<Option<Str
564577
}
565578

566579
pub fn wait_for_initial_scan(_: &Lua, timeout_ms: Option<u64>) -> LuaResult<bool> {
567-
let file_picker = FILE_PICKER
568-
.read()
569-
.with_lock_error(Error::AcquireItemLock)
570-
.into_lua_result()?;
571-
let picker = file_picker
572-
.as_ref()
573-
.ok_or(Error::FilePickerMissing)
574-
.into_lua_result()?;
580+
// Extract the scan signal Arc WITHOUT holding the read lock, so the
581+
// scan thread can acquire the write lock to store its results.
582+
// Holding a read lock while polling would deadlock: the scan thread
583+
// needs a write lock to finish, but can't acquire it while we hold the read lock.
584+
let scan_signal = {
585+
let file_picker = FILE_PICKER
586+
.read()
587+
.with_lock_error(Error::AcquireItemLock)
588+
.into_lua_result()?;
589+
let picker = file_picker
590+
.as_ref()
591+
.ok_or(Error::FilePickerMissing)
592+
.into_lua_result()?;
593+
picker.scan_signal()
594+
}; // read lock released here
575595

576596
let timeout_ms = timeout_ms.unwrap_or(500);
577597
let timeout_duration = Duration::from_millis(timeout_ms);
578598
let start_time = std::time::Instant::now();
579599
let mut sleep_duration = Duration::from_millis(1);
580600

581-
while picker.is_scan_active() {
601+
while scan_signal.load(std::sync::atomic::Ordering::Relaxed) {
582602
if start_time.elapsed() >= timeout_duration {
583603
::tracing::warn!("wait_for_initial_scan timed out after {}ms", timeout_ms);
584604
return Ok(false);

lua/fff/core.lua

Lines changed: 0 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -65,12 +65,6 @@ local function setup_global_autocmds(config)
6565
end,
6666
desc = 'Automatically sync FFF directory changes',
6767
})
68-
69-
vim.api.nvim_create_autocmd('VimLeavePre', {
70-
group = group,
71-
callback = function() pcall(fuzzy.cleanup_file_picker) end,
72-
desc = 'Cleanup FFF background threads on Neovim exit',
73-
})
7468
end
7569

7670
--- @return boolean

lua/fff/list_renderer.lua

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -189,17 +189,17 @@ local function apply_all_highlights(lines, item_to_lines, ctx, list_buf, ns_id)
189189
for i = ctx.display_start, ctx.display_end do
190190
local item = ctx.items[i]
191191
local item_lines = item_to_lines[i]
192-
if not item_lines then goto continue end
193192

194-
-- The content line is always the last line in the mapping
195-
local line_idx = item_lines.last
196-
local line_content = lines[line_idx]
193+
if item_lines then
194+
-- The content line is always the last line in the mapping
195+
local line_idx = item_lines.last
196+
local line_content = lines[line_idx]
197197

198-
if not line_content then goto continue end
199-
200-
---@diagnostic disable-next-line: param-type-mismatch
201-
renderer.apply_highlights(item, ctx, i, list_buf, ns_id, line_idx, line_content)
202-
::continue::
198+
if line_content then
199+
---@diagnostic disable-next-line: param-type-mismatch
200+
renderer.apply_highlights(item, ctx, i, list_buf, ns_id, line_idx, line_content)
201+
end
202+
end
203203
end
204204
end
205205

lua/fff/main.lua

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -42,6 +42,8 @@ function M.live_grep(opts)
4242
picker_ui.open(picker_opts)
4343
end
4444

45+
--- Changes the directory indexed by the file picker to the git root and opens the file picker
46+
--- @deprecated Use `find_files` instead
4547
function M.find_in_git_root()
4648
local fuzzy = require('fff.core').ensure_initialized()
4749
local ok, git_root = pcall(fuzzy.get_git_root)

0 commit comments

Comments
 (0)