fix: Crash if rescan triggered before bigramming is over

Author: dmtrKovalenko

Cargo.lock

@@ -34,6 +34,7 @@ bindet = { workspace = true }
 blake3 = { workspace = true }
 chrono = { workspace = true }
 dirs = { workspace = true }
+libc = "0.2"
 git2 = { workspace = true }
 glidesort = { workspace = true }
 globset = { workspace = true }

crates/fff-core/Cargo.toml

@@ -319,6 +319,15 @@ pub struct FilePicker {
     cancelled: Arc<AtomicBool>,
     bigram_index: Option<Arc<BigramFilter>>,
     bigram_overlay: Option<Arc<parking_lot::RwLock<BigramOverlay>>>,
+    // This is a soft lock that we use to prevent rescan be triggered while the 
+    // bigram indexing is in progress. This allows to keep some of the unsafe magic
+    // relying on the immutabillity of the files vec after the index without worrying
+    // that the vec is going to be dropped before the indexing is finished
+    //
+    // In addition to that rescan is likely triggered by something unnecessary 
+    // before the indexing is finished it means that fff is dogfooded the index either
+    // by the UI rendering preview or simply by walking the directory. Which is not good anyway
+    post_scan_busy: Arc<AtomicBool>,
 }
 
 impl std::fmt::Debug for FilePicker {
@@ -411,6 +420,7 @@ impl FilePicker {
             has_explicit_cache_budget: has_explicit_budget,
             is_scanning: Arc::new(AtomicBool::new(false)),
             mode: options.mode,
+            post_scan_busy: Arc::new(AtomicBool::new(false)),
             scanned_files_count: Arc::new(AtomicUsize::new(0)),
             sync_data: FileSync::new(),
             warmup_mmap_cache: options.warmup_mmap_cache,
@@ -445,6 +455,7 @@ impl FilePicker {
         let watcher_ready = Arc::clone(&picker.watcher_ready);
         let synced_files_count = Arc::clone(&picker.scanned_files_count);
         let cancelled = Arc::clone(&picker.cancelled);
+        let post_scan_busy = Arc::clone(&picker.post_scan_busy);
         let path = picker.base_path.clone();
 
         {
@@ -463,6 +474,7 @@ impl FilePicker {
             shared_picker,
             shared_frecency,
             cancelled,
+            post_scan_busy,
         );
 
         Ok(())
@@ -908,6 +920,14 @@ impl FilePicker {
             return Ok(());
         }
 
+        // The post-scan warmup + bigram phase holds a raw pointer into the
+        // current files Vec. Replacing sync_data now would free that memory.
+        // Skip — the background watcher will retry on the next event.
+        if self.post_scan_busy.load(Ordering::Acquire) {
+            debug!("Post-scan bigram build in progress, skipping rescan");
+            return Ok(());
+        }
+
         self.is_scanning.store(true, Ordering::Relaxed);
         self.scanned_files_count.store(0, Ordering::Relaxed);
 
@@ -1003,6 +1023,7 @@ fn spawn_scan_and_watcher(
     shared_picker: SharedPicker,
     shared_frecency: SharedFrecency,
     cancelled: Arc<AtomicBool>,
+    post_scan_busy: Arc<AtomicBool>,
 ) {
     std::thread::spawn(move || {
         // scan_signal is already `true` (set by the caller before spawning)
@@ -1094,6 +1115,7 @@ fn spawn_scan_and_watcher(
         watcher_ready.store(true, Ordering::Release);
 
         if warmup_mmap_cache && !cancelled.load(Ordering::Acquire) {
+            post_scan_busy.store(true, Ordering::Release);
             let phase_start = std::time::Instant::now();
 
             // Scale cache limits based on repo size (skip if caller provided an explicit budget).
@@ -1110,7 +1132,9 @@ fn spawn_scan_and_watcher(
             }
 
             // SAFETY: The file index Vec is not resized between the initial scan
-            // completing and the warmup + bigram phase finishing.
+            // completing and the warmup + bigram phase finishing because
+            // `post_scan_busy` prevents concurrent rescans from replacing
+            // sync_data while we hold the raw pointer.
             let files_snapshot: Option<(&[FileItem], Arc<ContentCacheBudget>)> =
                 if !cancelled.load(Ordering::Acquire) {
                     let guard = shared_picker.read().ok();
@@ -1120,7 +1144,9 @@ fn spawn_scan_and_watcher(
                             let ptr = files.as_ptr();
                             let len = files.len();
                             let budget = Arc::clone(&picker.cache_budget);
-                            // SAFETY: see comment above — Vec is stable during this window.
+                            // SAFETY: post_scan_busy flag blocks trigger_rescan and
+                            // background watcher rescans from replacing sync_data,
+                            // so the Vec backing this slice stays alive.
                             let static_files: &[FileItem] =
                                 unsafe { std::slice::from_raw_parts(ptr, len) };
                             (static_files, budget)
@@ -1171,6 +1197,8 @@ fn spawn_scan_and_watcher(
                 }
             }
 
+            post_scan_busy.store(false, Ordering::Release);
+
             info!(
                 "Post-scan warmup + bigram total: {:.2}s",
                 phase_start.elapsed().as_secs_f64(),

crates/fff-core/src/file_picker.rs

@@ -1,93 +1,82 @@
 //! Shared logging utilities for FFF crates.
 //!
-//! Provides file-based tracing initialization and a panic hook that writes
-//! to both stderr and a fallback log file.
+//! Provides file-based tracing initialization and crash handlers (panic hook
+//! + SIGSEGV signal handler) that write diagnostics to both stderr and the
+//! configured log file.
 
 use std::io;
-use std::path::Path;
+use std::path::{Path, PathBuf};
 use tracing_appender::non_blocking;
 use tracing_subscriber::fmt::format::FmtSpan;
 use tracing_subscriber::{EnvFilter, fmt, prelude::*};
 
 static TRACING_INITIALIZED: std::sync::OnceLock<tracing_appender::non_blocking::WorkerGuard> =
     std::sync::OnceLock::new();
 
-static PANIC_HOOK_INSTALLED: std::sync::OnceLock<()> = std::sync::OnceLock::new();
+static CRASH_HANDLERS_INSTALLED: std::sync::OnceLock<()> = std::sync::OnceLock::new();
 
-/// Install panic hook that writes to both stderr and a fallback file.
-/// This is called separately from init_tracing to ensure panics are always logged.
+/// The log file path set by `init_tracing`. Crash handlers append to this file.
+static LOG_FILE_PATH: std::sync::OnceLock<PathBuf> = std::sync::OnceLock::new();
+
+fn write_crash_report(header: &str, body: &str) {
+    let msg = format!("\n=== CRASH {} ===\n{}\n=== CRASH END {} ===\n", header, body, header);
+
+    let _ = std::io::Write::write_all(&mut std::io::stderr(), msg.as_bytes());
+
+    if let Some(path) = LOG_FILE_PATH.get() {
+        let _ = std::fs::OpenOptions::new()
+            .create(true)
+            .append(true)
+            .open(path)
+            .and_then(|mut f| std::io::Write::write_all(&mut f, msg.as_bytes()));
+    }
+}
+
+extern "C" fn sigsegv_handler(sig: libc::c_int) {
+    let bt = std::backtrace::Backtrace::force_capture();
+    write_crash_report("SIGSEGV", &format!("signal {}\n{}", sig, bt));
+
+    unsafe {
+        libc::signal(sig, libc::SIG_DFL);
+        libc::raise(sig);
+    }
+}
+
+/// Install both the panic hook and the SIGSEGV signal handler.
 pub fn install_panic_hook() {
-    PANIC_HOOK_INSTALLED.get_or_init(|| {
+    CRASH_HANDLERS_INSTALLED.get_or_init(|| {
         let default_panic = std::panic::take_hook();
-
         std::panic::set_hook(Box::new(move |panic_info| {
-            let payload = panic_info.payload();
-            let message = if let Some(s) = payload.downcast_ref::<&str>() {
+            let message = if let Some(s) = panic_info.payload().downcast_ref::<&str>() {
                 s.to_string()
-            } else if let Some(s) = payload.downcast_ref::<String>() {
+            } else if let Some(s) = panic_info.payload().downcast_ref::<String>() {
                 s.clone()
             } else {
                 "Unknown panic payload".to_string()
             };
 
-            let location = if let Some(location) = panic_info.location() {
-                format!(
-                    "{}:{}:{}",
-                    location.file(),
-                    location.line(),
-                    location.column()
-                )
-            } else {
-                "unknown location".to_string()
-            };
+            let location = panic_info
+                .location()
+                .map(|l| format!("{}:{}:{}", l.file(), l.line(), l.column()))
+                .unwrap_or_else(|| "unknown location".to_string());
 
-            // Always log to tracing (if initialized)
             tracing::error!(
                 panic.message = %message,
                 panic.location = %location,
                 "PANIC occurred in FFF"
             );
 
-            // Always print to stderr
-            eprintln!("=== FFF PANIC ===");
-            eprintln!("Message: {}", message);
-            eprintln!("Location: {}", location);
-            eprintln!("=================");
-
-            // Try to write to fallback panic log file
-            if let Some(cache_dir) = dirs::cache_dir() {
-                let panic_log = cache_dir.join("fff_panic.log");
-                let timestamp = std::time::SystemTime::now()
-                    .duration_since(std::time::UNIX_EPOCH)
-                    .map(|d| d.as_secs())
-                    .unwrap_or(0);
-
-                let panic_entry = format!(
-                    "\n[{}] PANIC at {}\nMessage: {}\n",
-                    timestamp, location, message
-                );
-
-                let _ = std::fs::OpenOptions::new()
-                    .create(true)
-                    .append(true)
-                    .open(&panic_log)
-                    .and_then(|mut f| {
-                        use std::io::Write;
-                        f.write_all(panic_entry.as_bytes())
-                    });
-
-                eprintln!("Panic logged to: {}", panic_log.display());
-            }
-
+            write_crash_report("PANIC", &format!("Message: {}\nLocation: {}", message, location));
             default_panic(panic_info);
         }));
+
+        unsafe {
+            libc::signal(libc::SIGSEGV, sigsegv_handler as *const () as libc::sighandler_t);
+        }
     });
 }
 
 /// Parse a log level string into a `tracing::Level`.
-///
-/// Accepts "trace", "debug", "info", "warn", "error" (case-insensitive).
-/// Returns `tracing::Level::INFO` for unrecognised values.
 pub fn parse_log_level(level: Option<&str>) -> tracing::Level {
     match level.as_ref().map(|s| s.trim().to_lowercase()).as_deref() {
         Some("trace") => tracing::Level::TRACE,
@@ -100,29 +89,19 @@ pub fn parse_log_level(level: Option<&str>) -> tracing::Level {
 }
 
 /// Initialize tracing with a single log file.
-///
-/// Creates the parent directory if it doesn't exist, truncates the log file,
-/// and sets up a non-blocking file appender with structured formatting.
-///
-/// # Arguments
-/// * `log_file_path` - Full path to the log file
-/// * `log_level` - Log level (trace, debug, info, warn, error)
-///
-/// # Returns
-/// * `Result<String, io::Error>` - Full path to the log file on success
 pub fn init_tracing(log_file_path: &str, log_level: Option<&str>) -> Result<String, io::Error> {
-    // Install panic hook first (does nothing if already installed)
-    install_panic_hook();
-
     let log_path = Path::new(log_file_path);
     if let Some(parent) = log_path.parent() {
         std::fs::create_dir_all(parent)?;
     }
 
+    let _ = LOG_FILE_PATH.set(log_path.to_path_buf());
+    install_panic_hook();
+
     let file_appender = std::fs::OpenOptions::new()
         .create(true)
         .write(true)
-        .truncate(true) // creates a new file on every setup
+        .truncate(true)
         .open(log_path)?;
 
     let level = parse_log_level(log_level);
@@ -137,8 +116,6 @@ pub fn init_tracing(log_file_path: &str, log_level: Option<&str>) -> Result<Stri
                     .with_target(true)
                     .with_thread_ids(false)
                     .with_thread_names(false)
-                    // .with_file(true)
-                    // .with_line_number(true)
                     .with_ansi(false)
                     .with_span_events(FmtSpan::NEW | FmtSpan::CLOSE),
             )