Skip to content

fix: Crash if rescan triggered before bigramming is over #349

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
dmtrKovalenko merged 1 commit into from
Apr 7, 2026
Merged

fix: Crash if rescan triggered before bigramming is over #349

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
1 change: 1 addition & 0 deletions Cargo.lock
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions crates/fff-core/Cargo.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 }
Expand Down
32 changes: 30 additions & 2 deletions crates/fff-core/src/file_picker.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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();

{
Expand All @@ -463,6 +474,7 @@ impl FilePicker {
shared_picker,
shared_frecency,
cancelled,
post_scan_busy,
);

Ok(())
Expand Down Expand Up @@ -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);

Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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).
Expand All @@ -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();
Expand All @@ -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)
Expand Down Expand Up @@ -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(),
Expand Down
130 changes: 58 additions & 72 deletions crates/fff-core/src/log.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,93 +1,91 @@
//! 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(
"RUST 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,
Expand All @@ -100,29 +98,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) // truncates a file on restart (instead of appending)
.open(log_path)?;

let level = parse_log_level(log_level);
Expand All @@ -137,8 +125,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),
)
Expand Down
Loading