gtauri: fix phantom rows, dead deletion detection, 5-min launch stall
Root causes found by reproducing each on the emulator (2.8k-file fixture with mixed jpg/mp4): 1. walk_dir_nostats (incremental rescan of >2000-file dirs) had NO image-extension filter: it re-enumerated every file, injecting all ~1,727 videos into the DB. This is what made the count swing 8,439 -> 10,166 and the grid full of blank cells (404 rows). Now filters through is_image_path() like every other walk. 2. walk_dir aborted the ENTIRE walk on any per-file stat failure (file deleted mid-walk), so an error during the walk meant the walk was never applied and deletions were never detected. Per-file stat errors are now skipped; only readdir/root errors fail the walk (an errored walk is still not applied). 3. dir_sig stat'ed every entry of every root dir every watch tick: 10k+ stats/second on the phone = 10s+ of FUSE I/O per second = the long delays. On Android the MediaStore URI payloads already carry the exact change signal, so the mtime-diff machinery now runs desktop-only. The Android watch tick is a single read_dir of our own app dir (no-op when empty): 1s backstop, 30s-throttled fallback full rescan for events with no payload, 5-min full pass. 4. TOCTOU: a delete consumed while a full rescan was mid-walk lost to the walk's stale snapshot (walk re-added the row). RESCANNING gate parks payloads during the walk; they apply after it, so mid-walk deletions always win. Reproduced deterministically (churn during the startup walk), fixed, re-verified. Emulator stress (all passing): full-walk convergence, add +1s, delete +1s, video add/delete = zero rows, 60-op churn converges to exact disk count, 3x relaunch idempotency, startup-walk race converges (1s normally, 5-min backstop when the OS scanner drops burst events), no-blank-cells visual check, 0.00% dropped frames. Phone verified: 10,166 -> 8,439 on first launch (all video phantoms purged), add appears ~2s after the OS indexes it, delete same, DB returns to 8,439 exactly. ContentObserver registration + onChange logging added (it fired reliably once the stale instance was replaced).
This commit is contained in:
parent
0ade19ee76
commit
10ec0b52cc
|
|
@ -164,6 +164,7 @@ class MainActivity : TauriActivity() {
|
|||
// Event-driven: the OS passes the changed URI(s) directly.
|
||||
val obs = object : ContentObserver(mainHandler) {
|
||||
override fun onChange(selfChange: Boolean, uri: Uri?, flags: Int) {
|
||||
android.util.Log.i("gallery-ms", "onChange uri=$uri self=$selfChange flags=$flags")
|
||||
if (selfChange) return
|
||||
fsExecutor.execute { handleUri(uri) }
|
||||
}
|
||||
|
|
@ -174,6 +175,7 @@ class MainActivity : TauriActivity() {
|
|||
contentResolver.registerContentObserver(
|
||||
MediaStore.Images.Media.EXTERNAL_CONTENT_URI, true, obs
|
||||
)
|
||||
android.util.Log.i("gallery-ms", "observer registered in pid " + android.os.Process.myPid())
|
||||
// Startup catch-up: if the app was killed after a photo was taken,
|
||||
// the observer won't replay — push whatever was indexed recently.
|
||||
mainHandler.postDelayed({ fsExecutor.execute { windowQuery() } }, 1500)
|
||||
|
|
@ -181,6 +183,7 @@ class MainActivity : TauriActivity() {
|
|||
}
|
||||
|
||||
override fun onDestroy() {
|
||||
android.util.Log.i("gallery-ms", "onDestroy: unregistering observer")
|
||||
observer?.let { contentResolver.unregisterContentObserver(it) }
|
||||
fsExecutor.shutdownNow()
|
||||
super.onDestroy()
|
||||
|
|
|
|||
|
|
@ -728,12 +728,34 @@ fn walk_once(app: &tauri::AppHandle) -> std::io::Result<Vec<scan::Entry>> {
|
|||
})
|
||||
}
|
||||
|
||||
/// Set while a full rescan's walk→apply is in flight. The watch loop
|
||||
/// must NOT consume MediaStore payloads meanwhile: a delete consumed
|
||||
/// mid-walk would race the walk's in-memory snapshot (the walk would
|
||||
/// re-add the row the delete already processed). Payloads simply wait
|
||||
/// as files and are drained by the rescan's post-apply drain.
|
||||
static RESCANNING: std::sync::atomic::AtomicBool =
|
||||
std::sync::atomic::AtomicBool::new(false);
|
||||
|
||||
struct RescanGuard;
|
||||
impl RescanGuard {
|
||||
fn new() -> Self {
|
||||
RESCANNING.store(true, std::sync::atomic::Ordering::Relaxed);
|
||||
Self
|
||||
}
|
||||
}
|
||||
impl Drop for RescanGuard {
|
||||
fn drop(&mut self) {
|
||||
RESCANNING.store(false, std::sync::atomic::Ordering::Relaxed);
|
||||
}
|
||||
}
|
||||
|
||||
/// Full rescan: walk the roots, diff against the DB (insert new /
|
||||
/// update changed / delete removed), refresh META, emit the list.
|
||||
/// Used by the startup scan AND the 5-minute reconciliation pass.
|
||||
/// If the walk fails, nothing is applied — an error is not "those
|
||||
/// files are gone"; the next pass retries.
|
||||
fn rescan(app: &tauri::AppHandle) {
|
||||
let _guard = RescanGuard::new();
|
||||
let entries = match walk_once(app) {
|
||||
Ok(e) => e,
|
||||
Err(e) => {
|
||||
|
|
@ -759,6 +781,11 @@ fn rescan(app: &tauri::AppHandle) {
|
|||
}
|
||||
emit_files(app, false, entries.len());
|
||||
log::info!("rescan: {} files", entries.len());
|
||||
// Draining happens in the watch loop's next tick: the RESCANNING
|
||||
// flag lifted at drop time guarantees any payload consumed from now
|
||||
// on is applied AFTER this walk's apply, so mid-walk deletions
|
||||
// always win over the stale snapshot. (The 5-min pass catches any
|
||||
// straggler in the worst-case millisecond window.)
|
||||
}
|
||||
|
||||
fn spawn_scan(app: tauri::AppHandle) {
|
||||
|
|
@ -774,9 +801,12 @@ fn mtime_ns(m: &std::fs::Metadata) -> i64 {
|
|||
}
|
||||
|
||||
/// Tree signature: (dir, mtime) for every directory under the roots.
|
||||
/// Verified on the phone: Android's FUSE DOES update a directory's mtime
|
||||
/// when files are added/removed inside it (stat before/after a copy
|
||||
/// showed the change). Stat-only cost: a few dozen syscalls per tick.
|
||||
/// Verified on the phone: Android's FUSE DOES update a directory's
|
||||
/// mtime when files are added/removed inside it (stat before/after a
|
||||
/// copy showed the change). Cost per tick: one stat per DIRECTORY and
|
||||
/// one readdir per directory — no per-FILE stat. (The old version
|
||||
/// called is_dir() on every entry: 10k stats per second on a 10k-file
|
||||
/// Camera dir, which put a 10 s I/O floor under every event.)
|
||||
fn dir_sig(roots: &[PathBuf]) -> HashMap<PathBuf, i64> {
|
||||
let mut out = HashMap::new();
|
||||
let mut stack: Vec<PathBuf> = roots.iter().filter(|r| r.is_dir()).cloned().collect();
|
||||
|
|
@ -787,7 +817,10 @@ fn dir_sig(roots: &[PathBuf]) -> HashMap<PathBuf, i64> {
|
|||
Ok(rd) => {
|
||||
for ent in rd.flatten() {
|
||||
let p = ent.path();
|
||||
if p.is_symlink() || !p.is_dir() {
|
||||
// file_type() uses the d_type from getdents (no
|
||||
// syscall); is_dir() would stat the entry.
|
||||
let Ok(ft) = ent.file_type() else { continue };
|
||||
if ft.is_symlink() || !ft.is_dir() {
|
||||
continue;
|
||||
}
|
||||
if p.file_name().map_or(false, |n| n.to_string_lossy().starts_with('.')) {
|
||||
|
|
@ -902,11 +935,14 @@ fn rescan_subtrees(app: &tauri::AppHandle, changed: &[PathBuf]) {
|
|||
// A|<media_id>|<path>|<size> row exists → upsert
|
||||
// D|<media_id> row gone → delete by id
|
||||
// ---------------------------------------------------------------------
|
||||
/// Consume pending event files. Returns true when at least one change
|
||||
/// line was applied (the watch loop uses this to decide whether the
|
||||
/// OS event was fully handled by the payload path).
|
||||
#[cfg(target_os = "android")]
|
||||
fn apply_mediasotre_payload(app: &tauri::AppHandle) {
|
||||
fn apply_mediasotre_payload(app: &tauri::AppHandle) -> bool {
|
||||
// Consume every pending .ms_recent.* event file (oldest first).
|
||||
let dir = external_files_dir();
|
||||
let Ok(rd) = std::fs::read_dir(&dir) else { return };
|
||||
let Ok(rd) = std::fs::read_dir(&dir) else { return false };
|
||||
let mut files: Vec<std::path::PathBuf> = rd
|
||||
.flatten()
|
||||
.filter(|e| {
|
||||
|
|
@ -917,9 +953,10 @@ fn apply_mediasotre_payload(app: &tauri::AppHandle) {
|
|||
.map(|e| e.path())
|
||||
.collect();
|
||||
if files.is_empty() {
|
||||
return;
|
||||
return false;
|
||||
}
|
||||
files.sort();
|
||||
let mut applied = false;
|
||||
for f in files {
|
||||
let name = f.file_name().map(|s| s.to_string_lossy().into_owned()).unwrap_or_default();
|
||||
let Ok(text) = std::fs::read_to_string(&f) else { continue };
|
||||
|
|
@ -928,20 +965,23 @@ fn apply_mediasotre_payload(app: &tauri::AppHandle) {
|
|||
continue;
|
||||
}
|
||||
tr(&format!("mediastore event {name}: {} lines", text.lines().count()));
|
||||
mediastore_apply(app, &text);
|
||||
applied |= mediastore_apply(app, &text);
|
||||
}
|
||||
applied
|
||||
}
|
||||
|
||||
#[cfg(not(target_os = "android"))]
|
||||
fn apply_mediasotre_payload(_app: &tauri::AppHandle) {}
|
||||
fn apply_mediasotre_payload(_app: &tauri::AppHandle) -> bool {
|
||||
false
|
||||
}
|
||||
|
||||
#[cfg(target_os = "android")]
|
||||
fn mediastore_apply(app: &tauri::AppHandle, text: &str) {
|
||||
fn mediastore_apply(app: &tauri::AppHandle, text: &str) -> bool {
|
||||
let mut conn = match db::open(&db::db_path()) {
|
||||
Ok(c) => c,
|
||||
Err(e) => {
|
||||
log::warn!("mediastore db: {e}");
|
||||
return;
|
||||
return false;
|
||||
}
|
||||
};
|
||||
let mut changed = 0usize;
|
||||
|
|
@ -1038,6 +1078,7 @@ fn mediastore_apply(app: &tauri::AppHandle, text: &str) {
|
|||
}
|
||||
emit_files(app, false, META.read().unwrap().len());
|
||||
}
|
||||
changed + removed > 0
|
||||
}
|
||||
|
||||
/// Watcher wake events.
|
||||
|
|
@ -1140,7 +1181,13 @@ fn watch_files(app: tauri::AppHandle) {
|
|||
});
|
||||
#[cfg(not(target_os = "android"))]
|
||||
spawn_fs_watcher(tx);
|
||||
// Desktop only: the mtime-diff machinery. On Android the MediaStore
|
||||
// URI payloads ARE the change signal (exact paths, no scanning),
|
||||
// and diffing would cost a 10k-entry readdir every second on the
|
||||
// phone — pure overhead for what the payloads already deliver.
|
||||
#[cfg(not(target_os = "android"))]
|
||||
let mut sig: HashMap<PathBuf, i64> = HashMap::new();
|
||||
#[cfg(not(target_os = "android"))]
|
||||
let mut baseline = true;
|
||||
let mut last_full = std::time::Instant::now();
|
||||
loop {
|
||||
|
|
@ -1156,12 +1203,42 @@ fn watch_files(app: tauri::AppHandle) {
|
|||
std::thread::sleep(std::time::Duration::from_millis(600));
|
||||
while rx.try_recv().is_ok() {}
|
||||
}
|
||||
// MediaStore event path: MainActivity drops a one-line payload
|
||||
// per changed URI and wakes us via nativeFsChange. Consume on
|
||||
// EVERY tick — the check is a single read_dir of our own app
|
||||
// dir (no-op when empty), so a lost JNI wake self-heals within
|
||||
// the 1 s backstop instead of waiting for the 5-min pass.
|
||||
apply_mediasotre_payload(&app);
|
||||
if do_full {
|
||||
tr("watch: full rescan (5min)");
|
||||
rescan(&app);
|
||||
last_full = std::time::Instant::now();
|
||||
continue;
|
||||
}
|
||||
// MediaStore event path (Android): MainActivity drops a one-line
|
||||
// payload per changed URI and wakes us via nativeFsChange.
|
||||
// Consume on EVERY tick (single read_dir of our own app dir —
|
||||
// no-op when empty, so a lost JNI wake self-heals within the 1 s
|
||||
// backstop) — but NOT while a full rescan is mid-walk: those
|
||||
// payloads wait as files and apply after the walk's apply, so
|
||||
// mid-walk deletions win over the walk's stale snapshot.
|
||||
let applied_ms = if RESCANNING.load(std::sync::atomic::Ordering::Relaxed) {
|
||||
false
|
||||
} else {
|
||||
apply_mediasotre_payload(&app)
|
||||
};
|
||||
// Android fallback: the OS said "media changed" but no payload
|
||||
// arrived (permission missing, scanner gap, or the payload was
|
||||
// parked behind a rescan). Full rescan, throttled — the
|
||||
// degraded path, not the common one. Parking behind a rescan
|
||||
// also counts as "not applied": the rescan's next 1 s tick
|
||||
// re-evaluates and either drains the payload or rescans.
|
||||
#[cfg(target_os = "android")]
|
||||
if matches!(woke, Some(WEvent::Fs)) && !applied_ms
|
||||
&& last_full.elapsed() >= std::time::Duration::from_secs(30)
|
||||
{
|
||||
tr("watch: full rescan (event, no payload)");
|
||||
rescan(&app);
|
||||
last_full = std::time::Instant::now();
|
||||
continue;
|
||||
}
|
||||
// Desktop: diff the dir-mtime signature, rescan changed subtrees.
|
||||
#[cfg(not(target_os = "android"))]
|
||||
{
|
||||
let next = dir_sig(&scan::roots());
|
||||
let changed = if baseline {
|
||||
baseline = false;
|
||||
|
|
@ -1170,29 +1247,13 @@ fn watch_files(app: tauri::AppHandle) {
|
|||
changed_dirs(&sig, &next)
|
||||
};
|
||||
sig = next;
|
||||
if do_full {
|
||||
tr("watch: full rescan (5min)");
|
||||
rescan(&app);
|
||||
last_full = std::time::Instant::now();
|
||||
continue;
|
||||
}
|
||||
if changed.is_empty() {
|
||||
if woke.is_some() {
|
||||
// The OS reported a change but no dir mtime moved
|
||||
// (in-place edit / event for an untracked file):
|
||||
// full rescan, throttled to one per minute.
|
||||
if last_full.elapsed() >= std::time::Duration::from_secs(60) {
|
||||
tr("watch: full rescan (event, no dir delta)");
|
||||
rescan(&app);
|
||||
last_full = std::time::Instant::now();
|
||||
}
|
||||
}
|
||||
// Timeout with no dir delta: nothing to do (~0.05 ms spent).
|
||||
continue;
|
||||
}
|
||||
tr(&format!("watch: {} changed dir(s)", changed.len()));
|
||||
rescan_subtrees(&app, &changed);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
// ---------------------------------------------------------------------------
|
||||
|
|
|
|||
|
|
@ -3,10 +3,18 @@
|
|||
//! Plain recursive scan of the roots (Pictures + DCIM on Android).
|
||||
//! FUSE is a reliable infrastructure point: a directory reads either
|
||||
//! returns its full listing or returns an error. There is no silent
|
||||
//! truncation. Therefore the one invariant here is simple and strict:
|
||||
//! **any read_dir or stat error fails the walk** — a failed walk must
|
||||
//! never be applied to the DB, because an error is not "those files
|
||||
//! are gone."
|
||||
//! truncation. Error semantics:
|
||||
//! - a **read_dir error fails the walk** — the result would be
|
||||
//! incomplete and must never be applied to the DB (an error is not
|
||||
//! "those files are gone");
|
||||
//! - a **per-file failure is skipped** — a file vanishing between
|
||||
//! readdir and stat is a normal race on an actively used
|
||||
//! directory (the user deleting while we walk), not a walk
|
||||
//! failure. Aborting on this killed deletion detection: every
|
||||
//! incremental rescan of a busy 10k-file directory aborted.
|
||||
//! Both walks (with-stat and readdir-only) filter to image
|
||||
//! extensions: library directories hold thousands of videos, and
|
||||
//! enumerating them would inject non-image rows into the DB.
|
||||
|
||||
use std::io;
|
||||
use std::path::{Path, PathBuf};
|
||||
|
|
@ -108,8 +116,13 @@ pub fn walk_dir_into(dir: &Path, out: &mut Vec<Entry>) -> io::Result<()> {
|
|||
/// Readdir-only walk (NO per-file stat) of ONE directory subtree — for
|
||||
/// huge FUSE directories where stat'ing every file costs ~30 s; the
|
||||
/// caller supplies metadata for new files and keeps DB metadata for
|
||||
/// known ones. A read_dir error fails the walk; a per-file type
|
||||
/// failure (file vanishing mid-walk — a normal race) is skipped.
|
||||
/// known ones.
|
||||
///
|
||||
/// FILTERS TO IMAGE EXTENSIONS (critical: a library directory holds
|
||||
/// thousands of videos too — enumerating them would inject non-image
|
||||
/// rows into the DB: phantom blank grid cells and count swings).
|
||||
/// A read_dir error fails the walk; per-file failures (file vanishing
|
||||
/// mid-walk — a normal race) are skipped.
|
||||
pub fn walk_dir_nostats(dir: &Path, cb: &mut dyn FnMut(&Path)) -> io::Result<()> {
|
||||
if !dir.is_dir() {
|
||||
return Ok(());
|
||||
|
|
@ -129,13 +142,20 @@ pub fn walk_dir_nostats(dir: &Path, cb: &mut dyn FnMut(&Path)) -> io::Result<()>
|
|||
}
|
||||
stack.push(p);
|
||||
} else if ft.is_file() {
|
||||
let ps = p.to_string_lossy();
|
||||
if is_image_path(&ps) {
|
||||
cb(&p);
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
Ok(())
|
||||
}
|
||||
|
||||
/// Full stat walk. Error semantics: a read_dir error fails the walk
|
||||
/// (the result would be incomplete — must never be applied); per-file
|
||||
/// failures (a file vanishing between readdir and stat — a normal race
|
||||
/// on an actively used directory) are SKIPPED, not propagated.
|
||||
fn walk_dir(
|
||||
dir: &Path,
|
||||
out: &mut Vec<Entry>,
|
||||
|
|
@ -144,7 +164,7 @@ fn walk_dir(
|
|||
for ent in std::fs::read_dir(dir)? {
|
||||
let ent = ent?;
|
||||
let p = ent.path();
|
||||
let ft = ent.file_type()?;
|
||||
let Ok(ft) = ent.file_type() else { continue };
|
||||
if ft.is_dir() {
|
||||
// Skip hidden dirs (.thumbnails, .trash, …) — metadata, not photos.
|
||||
if p.file_name().and_then(|n| n.to_str()).is_some_and(|n| n.starts_with('.')) {
|
||||
|
|
@ -164,7 +184,7 @@ fn walk_dir(
|
|||
if !is_image_file(name, ext) {
|
||||
continue;
|
||||
}
|
||||
let (mtime, size) = file_metadata(&p)?;
|
||||
let Ok((mtime, size)) = file_metadata(&p) else { continue };
|
||||
out.push(Entry {
|
||||
path: p.to_string_lossy().into_owned(),
|
||||
mtime,
|
||||
|
|
|
|||
Loading…
Reference in New Issue
Block a user