From ca7452ae6ce323ec2b2a615cd413063fa5b7ac44 Mon Sep 17 00:00:00 2001 From: Abdelkader Boudih Date: Sat, 20 Jun 2026 22:34:09 +0100 Subject: [PATCH] refactor(core): generic SharedDb replaces the two LMDB handles SharedFrecency and SharedQueryTracker are now type aliases over a single SharedDb, and the three wait_for_* methods share one poll_until helper. Public API unchanged. --- crates/fff-core/src/shared.rs | 174 +++++++++++----------------------- 1 file changed, 57 insertions(+), 117 deletions(-) diff --git a/crates/fff-core/src/shared.rs b/crates/fff-core/src/shared.rs index f88dc3ba..9da70ab8 100644 --- a/crates/fff-core/src/shared.rs +++ b/crates/fff-core/src/shared.rs @@ -2,7 +2,7 @@ use std::path::{Path, PathBuf}; use std::sync::{Arc, RwLock, RwLockReadGuard, RwLockWriteGuard, Weak}; use std::time::{Duration, Instant}; -use crate::dbs::lmdb::spawn_lmdb_gc; +use crate::dbs::lmdb::{LmdbStore, spawn_lmdb_gc}; use crate::error::Error; use crate::file_picker::FilePicker; use crate::frecency::FrecencyTracker; @@ -38,6 +38,19 @@ fn wait_for_git_index_lock_release(git_root: &Path) { } } +/// Poll `done` every 10ms until it returns `true`, or until `timeout` elapses. +/// Returns `true` if the condition was met, `false` on timeout. +fn poll_until(timeout: Duration, mut done: impl FnMut() -> bool) -> bool { + let start = Instant::now(); + while !done() { + if start.elapsed() >= timeout { + return false; + } + std::thread::sleep(Duration::from_millis(10)); + } + true +} + /// Thread-safe shared handle to the [`FilePicker`] instance. /// This accumulates only asynchronous non-blocking operations against the /// file picker: creating, triggering various rescans and so on. @@ -135,14 +148,9 @@ impl SharedFilePicker { } }; - let start = std::time::Instant::now(); - while signal.load(std::sync::atomic::Ordering::Acquire) { - if start.elapsed() >= timeout { - return false; - } - std::thread::sleep(Duration::from_millis(10)); - } - true + poll_until(timeout, || { + !signal.load(std::sync::atomic::Ordering::Acquire) + }) } /// Block until the background file watcher is ready. @@ -156,14 +164,9 @@ impl SharedFilePicker { } }; - let start = std::time::Instant::now(); - while !watch_ready_signal.load(std::sync::atomic::Ordering::Acquire) { - if start.elapsed() >= timeout { - return false; - } - std::thread::sleep(Duration::from_millis(10)); - } - true + poll_until(timeout, || { + watch_ready_signal.load(std::sync::atomic::Ordering::Acquire) + }) } /// Blocks until both the filesystem walk and post-scan indexing are done. @@ -180,18 +183,10 @@ impl SharedFilePicker { } }; - let start = std::time::Instant::now(); - loop { - if start.elapsed() >= timeout { - return false; - } - let s = scanning.load(std::sync::atomic::Ordering::Acquire); - let p = post_scan_active.load(std::sync::atomic::Ordering::Acquire); - if !s && !p { - return true; - } - std::thread::sleep(Duration::from_millis(10)); - } + poll_until(timeout, || { + !scanning.load(std::sync::atomic::Ordering::Acquire) + && !post_scan_active.load(std::sync::atomic::Ordering::Acquire) + }) } /// Trigger a full filesystem rescan without blocking the caller. @@ -262,14 +257,29 @@ impl SharedFilePicker { } } -/// Thread-safe shared handle to the [`FrecencyTracker`] instance. -#[derive(Clone)] -pub struct SharedFrecency { - inner: Arc>>, +/// Thread-safe shared handle to an LMDB-backed store. A disabled (`noop`) +/// instance silently ignores writes. See the [`SharedFrecency`] and +/// [`SharedQueryTracker`] aliases. +/// +/// `LmdbStore` is intentionally crate-private, so the store type is sealed: +/// only `FrecencyTracker` / `QueryTracker` can ever instantiate this. +#[allow(private_bounds)] +pub struct SharedDb { + inner: Arc>>, enabled: bool, } -impl Default for SharedFrecency { +// Hand-written to avoid a spurious `T: Clone` bound — `Arc` is always `Clone`. +impl Clone for SharedDb { + fn clone(&self) -> Self { + Self { + inner: self.inner.clone(), + enabled: self.enabled, + } + } +} + +impl Default for SharedDb { fn default() -> Self { Self { inner: Arc::new(RwLock::new(None)), @@ -278,13 +288,14 @@ impl Default for SharedFrecency { } } -impl std::fmt::Debug for SharedFrecency { +impl std::fmt::Debug for SharedDb { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { - f.debug_tuple("SharedFrecency").field(&"..").finish() + f.debug_tuple("SharedDb").field(&T::LABEL).finish() } } -impl SharedFrecency { +#[allow(private_bounds)] +impl SharedDb { /// Creates a disabled instance that silently ignores all writes. pub fn noop() -> Self { Self { @@ -293,15 +304,16 @@ impl SharedFrecency { } } - pub fn read(&self) -> Result>, Error> { + pub fn read(&self) -> Result>, Error> { self.inner.read().map_err(|_| Error::AcquireFrecencyLock) } - pub fn write(&self) -> Result>, Error> { + pub fn write(&self) -> Result>, Error> { self.inner.write().map_err(|_| Error::AcquireFrecencyLock) } - pub fn init(&self, tracker: FrecencyTracker) -> Result<(), Error> { + /// Initialize the store + spawn GC in the background. No-op when disabled. + pub fn init(&self, tracker: T) -> Result<(), Error> { if !self.enabled { return Ok(()); } @@ -330,7 +342,7 @@ impl SharedFrecency { let Some(tracker) = guard.take() else { return Ok(None); }; - let db_path = tracker.db_path().to_path_buf(); + let db_path = tracker.env().path().to_path_buf(); // Drop closes the LMDB env and unmaps the files drop(tracker); drop(guard); @@ -342,80 +354,8 @@ impl SharedFrecency { } } -/// Thread-safe shared handle to the [`QueryTracker`] instance. -#[derive(Clone)] -pub struct SharedQueryTracker { - inner: Arc>>, - enabled: bool, -} - -impl Default for SharedQueryTracker { - fn default() -> Self { - Self { - inner: Arc::new(RwLock::new(None)), - enabled: true, - } - } -} - -impl std::fmt::Debug for SharedQueryTracker { - fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { - f.debug_tuple("SharedQueryTracker").field(&"..").finish() - } -} - -impl SharedQueryTracker { - /// Creates a disabled instance that silently ignores all writes. - pub fn noop() -> Self { - Self { - inner: Arc::new(RwLock::new(None)), - enabled: false, - } - } - - pub fn read(&self) -> Result>, Error> { - self.inner.read().map_err(|_| Error::AcquireFrecencyLock) - } - - pub fn write(&self) -> Result>, Error> { - self.inner.write().map_err(|_| Error::AcquireFrecencyLock) - } - - /// Initialize the query tracker + spawn GC in the background. - /// No-op if this is a disabled instance. - pub fn init(&self, tracker: QueryTracker) -> Result<(), Error> { - if !self.enabled { - return Ok(()); - } - { - let mut guard = self.write()?; - *guard = Some(tracker); - } - - spawn_lmdb_gc(self.inner.clone()); - Ok(()) - } +/// Thread-safe shared handle to the [`FrecencyTracker`] instance. +pub type SharedFrecency = SharedDb; - ///Drop the in-memory tracker and delete the on-disk database directory. - /// - /// Acquires the write lock, ensuring all readers (including any active mmap - /// access) are finished before the LMDB environment is closed and the files - /// are removed. - /// - /// Returns `Ok(Some(path))` with the deleted path, or `Ok(None)` if no - /// tracker was initialized. - pub fn destroy(&self) -> Result, Error> { - let mut guard = self.write()?; - let Some(tracker) = guard.take() else { - return Ok(None); - }; - let db_path = tracker.db_path().to_path_buf(); - drop(tracker); - drop(guard); - std::fs::remove_dir_all(&db_path).map_err(|source| Error::RemoveDbDir { - path: db_path.clone(), - source, - })?; - Ok(Some(db_path)) - } -} +/// Thread-safe shared handle to the [`QueryTracker`] instance. +pub type SharedQueryTracker = SharedDb;