Merge branch 'fix/diagnose-coverage' into fix/restore-outcome-and-walk-gaps

# Conflicts:
#	docs/BUGS.md

crates/pdfs-fuse/src/diagnostics.rs

@@ -163,6 +163,7 @@ impl Core {
             control_handlers: super::control::active_handlers(),
             control_handler_limit: super::control::handler_limit(),
             pending_ops: self.pending_ops_quickly(),
+            sync: Some(self.sync_engine.report()),
         }
     }
 

crates/pdfs-fuse/src/lib.rs

@@ -124,7 +124,7 @@ use mount::{
 };
 use reads::{BlockFlight, BlockRing, PREFETCH_BUDGET, Prefetch, ReaderSlot, STREAM_BYPASS_MIN};
 use state::{Entry, Intervals, PendingRevision, State, StateGuard, WriteHandle};
-pub use systemd::idle_sleep;
+pub use systemd::{idle_sleep, status as notify_status};
 use tracing::{debug, error, info, warn};
 use transfers::{CountingWriter, JobGuard, OwnedCountingReader, TransferRegistry};
 use r#virtual::{
@@ -728,6 +728,8 @@ struct Core {
     /// can say what a pass is doing rather than just "syncing". An entry exists
     /// only while that folder's reconcile pass is running.
     sync_progress: Arc<Mutex<HashMap<i64, SyncProgress>>>,
+    /// Whether the folder-sync engine is alive, and which pass it is on.
+    sync_engine: Arc<sync::EngineHealth>,
     /// Channel to the folder-sync engine: nudges it to
     /// reconcile a folder, reconcile everything, or re-scan its watch set.
     sync_tx: std::sync::mpsc::Sender<sync::SyncMsg>,
@@ -5989,6 +5991,7 @@ impl Core {
         if let Some(p) = self.sync_progress.lock().get_mut(&folder_id) {
             f(p);
         }
+        self.sync_engine.moved();
     }
 
     /// Set the number of items the scan expects to check, from the size of the

crates/pdfs-fuse/src/mount.rs

@@ -507,6 +507,7 @@ pub(crate) fn mount_with(
         events: Arc::new(Default::default()),
         indexing: Arc::new(AtomicBool::new(false)),
         sync_progress: Arc::new(Mutex::new(HashMap::new())),
+        sync_engine: Arc::default(),
         sync_tx,
         mounts: Arc::new(Mutex::new(SecondaryMountRegistry::default())),
         sync_locks: Arc::new(Mutex::new(HashMap::new())),
@@ -540,12 +541,11 @@ pub(crate) fn mount_with(
     // and sweeps all operating on a mount that no longer exists (`docs/BUGS.md` B44).
     let mut workers: Vec<std::thread::JoinHandle<()>> = Vec::new();
     for worker in 0..DRAIN_WORKERS {
-        let core = core.clone();
-        workers.push(
-            std::thread::Builder::new()
-                .name(format!("pdfs-drain-{worker}"))
-                .spawn(move || core.run_pending_drain(worker == 0))?,
-        );
+        let drain_core = core.clone();
+        let spawned = std::thread::Builder::new()
+            .name(format!("pdfs-drain-{worker}"))
+            .spawn(move || drain_core.run_pending_drain(worker == 0));
+        keep_or_abandon(&core, control_socket, &mut workers, spawned)?;
     }
 
     // Start the folder-sync engine. It watches every mirror folder, polls the
@@ -565,24 +565,22 @@ pub(crate) fn mount_with(
         info!("conflict sweep disabled by configuration");
     } else {
         info!(mode = ?sweep_mode, "conflict sweep enabled");
-        let core = core.clone();
-        workers.push(
-            std::thread::Builder::new()
-                .name("pdfs-conflict-sweep".into())
-                .spawn(move || core.run_conflict_sweep_loop())?,
-        );
+        let sweep_core = core.clone();
+        let spawned = std::thread::Builder::new()
+            .name("pdfs-conflict-sweep".into())
+            .spawn(move || sweep_core.run_conflict_sweep_loop());
+        keep_or_abandon(&core, control_socket, &mut workers, spawned)?;
     }
 
     // Watch the link for the life of the mount: a mount from the cache waits for
     // the network to come back, and an online one can lose it at any moment and
     // must notice it coming back just the same.
     {
-        let core = core.clone();
-        workers.push(
-            std::thread::Builder::new()
-                .name("pdfs-online-probe".into())
-                .spawn(move || core.run_online_probe())?,
-        );
+        let probe_core = core.clone();
+        let spawned = std::thread::Builder::new()
+            .name("pdfs-online-probe".into())
+            .spawn(move || probe_core.run_online_probe());
+        keep_or_abandon(&core, control_socket, &mut workers, spawned)?;
     }
 
     // Keep the launcher prompt's "This computer" index fresh. Its own thread:
@@ -593,11 +591,10 @@ pub(crate) fn mount_with(
         let transfers = core.transfers.clone();
         let mountpoint = mountpoint.to_path_buf();
         let shutdown = core.shutdown.clone();
-        workers.push(
-            std::thread::Builder::new()
-                .name("pdfs-localindex".into())
-                .spawn(move || run_local_index(db, indexing, transfers, mountpoint, shutdown))?,
-        );
+        let spawned = std::thread::Builder::new()
+            .name("pdfs-localindex".into())
+            .spawn(move || run_local_index(db, indexing, transfers, mountpoint, shutdown));
+        keep_or_abandon(&core, control_socket, &mut workers, spawned)?;
     }
 
     // Bind the control socket before the FUSE session takes over the thread. A
@@ -606,12 +603,18 @@ pub(crate) fn mount_with(
     let old_umask = unsafe { libc::umask(0o77) };
     let listener_res = UnixListener::bind(control_socket);
     unsafe { libc::umask(old_umask) };
-    let listener = listener_res?;
+    let listener = match listener_res {
+        Ok(listener) => listener,
+        Err(error) => {
+            abandon_workers(&core, control_socket, workers);
+            return Err(error);
+        }
+    };
     // Owner-only before anything can connect: a peer on this socket commands the
     // daemon's authenticated session without a credential of its own (B6).
     if let Err(e) = pdfs_core::config::restrict_socket(control_socket) {
         error!(error = %e, "cannot restrict control socket permissions; refusing to serve");
-        let _ = std::fs::remove_file(control_socket);
+        abandon_workers(&core, control_socket, workers);
         return Err(std::io::Error::other(format!(
             "control socket permissions: {e}"
         )));
@@ -622,7 +625,7 @@ pub(crate) fn mount_with(
     let bg = match spawn_session(&core, mountpoint, root) {
         Ok(session) => session,
         Err(error) => {
-            let _ = std::fs::remove_file(control_socket);
+            abandon_workers(&core, control_socket, workers);
             return Err(error);
         }
     };
@@ -642,7 +645,7 @@ pub(crate) fn mount_with(
             Err(error) => {
                 // A failure here abandons the mount, so the background threads
                 // started above have to be told to stop with it.
-                stop_workers(&core, control_socket, workers);
+                abandon_workers(&core, control_socket, workers);
                 let secondaries = core.mounts.lock().close_and_drain();
                 for (id, mount) in secondaries {
                     if let Err(unmount_error) = mount.teardown() {
@@ -692,8 +695,12 @@ pub(crate) fn mount_with(
         }
     }
     // The mount is live and the control socket answers: this is the moment the
-    // unit is actually usable, which is what `Type=notify` reports.
+    // unit is actually usable, which is what `Type=notify` reports. Whatever
+    // failure an earlier attempt recorded for the front ends is over.
     systemd::ready();
+    if let Some(state_dir) = control_socket.parent() {
+        pdfs_core::mountfail::clear(state_dir);
+    }
 
     // Re-establish on-demand mounts only after the primary session is live.
     // Keep the worker so shutdown can close publication, wait for any in-flight
@@ -859,6 +866,43 @@ fn stop_workers(core: &Core, control_socket: &Path, workers: Vec<std::thread::Jo
     // Say so before the joins: a teardown that takes a while must not be read as
     // a missed watchdog ping and turned into a kill.
     systemd::stopping();
+    halt_workers(core, control_socket, workers);
+}
+
+/// Stop the threads a mount attempt started before it failed.
+///
+/// The early returns used to leave them running: every retry started another
+/// set beside the last one's, and their `Core` kept the database open, so the
+/// next attempt could fail on its lock instead of the real error
+/// (`docs/BUGS.md` B264). Unlike [`stop_workers`] this does not tell systemd
+/// the daemon is stopping, because it is about to try again, and a manager
+/// told so would wait for the process to exit and then kill it.
+fn abandon_workers(core: &Core, control_socket: &Path, workers: Vec<std::thread::JoinHandle<()>>) {
+    halt_workers(core, control_socket, workers);
+}
+
+/// Keep a worker the mount just spawned, or, if the spawn failed, stop the
+/// ones already running and hand the error back.
+fn keep_or_abandon(
+    core: &Core,
+    control_socket: &Path,
+    workers: &mut Vec<std::thread::JoinHandle<()>>,
+    spawned: std::io::Result<std::thread::JoinHandle<()>>,
+) -> std::io::Result<()> {
+    match spawned {
+        Ok(handle) => {
+            workers.push(handle);
+            Ok(())
+        }
+        Err(error) => {
+            abandon_workers(core, control_socket, std::mem::take(workers));
+            Err(error)
+        }
+    }
+}
+
+/// Signal every worker to stop, then join them up to [`WORKER_JOIN_DEADLINE`].
+fn halt_workers(core: &Core, control_socket: &Path, workers: Vec<std::thread::JoinHandle<()>>) {
     core.shutdown.stop();
     core.wake_drain();
     let _ = core.sync_tx.send(sync::SyncMsg::Stop);

crates/pdfs-fuse/src/supervisor.rs

@@ -36,6 +36,11 @@ const TICK: Duration = Duration::from_secs(15);
 /// would train everyone to ignore the real one.
 const STALL_AFTER: Duration = Duration::from_secs(120);
 
+/// How long a sync pass may go without reporting progress before it is called
+/// out. Much longer than [`STALL_AFTER`]: one large file in a pass reports
+/// nothing until its whole transfer is done.
+const SYNC_STALL_AFTER: Duration = Duration::from_secs(900);
+
 /// How often resident size goes into the log.
 const RSS_EVERY: Duration = Duration::from_secs(300);
 
@@ -137,6 +142,26 @@ fn report_stalls(report: &pdfs_core::control::Diagnostics, last_completed: (u64,
             "the transfer lane has not completed a job since the last check"
         );
     }
+    // The engine is one thread for every synced folder, so a dead or stuck one
+    // stops all of them while the worker pool looks idle (B259).
+    if let Some(sync) = &report.sync {
+        if !sync.running {
+            stalled = true;
+            warn!(
+                "the sync engine is not running; synced folders do not sync until the daemon restarts"
+            );
+        } else if let Some(folder) = sync.pass_folder
+            && sync.pass_idle_secs >= SYNC_STALL_AFTER.as_secs()
+        {
+            stalled = true;
+            warn!(
+                folder,
+                age_secs = sync.pass_age_secs,
+                idle_secs = sync.pass_idle_secs,
+                "a sync pass has not reported progress for a long time"
+            );
+        }
+    }
     stalled
 }
 
@@ -169,7 +194,7 @@ fn probe(control_socket: &Path) -> bool {
 #[cfg(test)]
 mod tests {
     use super::*;
-    use pdfs_core::control::{Diagnostics, InflightRequest, WorkerState};
+    use pdfs_core::control::{Diagnostics, InflightRequest, SyncEngineState, WorkerState};
 
     fn busy_worker(age_secs: u64) -> WorkerState {
         WorkerState {
@@ -226,4 +251,45 @@ mod tests {
         assert!(!report_stalls(&report, (0, 30)));
         assert!(report_stalls(&report, (0, 40)));
     }
+
+    fn with_sync(sync: SyncEngineState) -> Diagnostics {
+        Diagnostics {
+            sync: Some(sync),
+            ..Default::default()
+        }
+    }
+
+    /// A sync engine that has ended stops every synced folder while the
+    /// workers look idle, which is what issue #53 looked like (B259).
+    #[test]
+    fn a_stopped_sync_engine_is_a_stall() {
+        assert!(report_stalls(
+            &with_sync(SyncEngineState::default()),
+            (0, 0)
+        ));
+    }
+
+    /// A pass that keeps reporting progress is working, however long it runs;
+    /// one that has gone quiet past the threshold is not.
+    #[test]
+    fn only_a_quiet_sync_pass_is_a_stall() {
+        let busy = SyncEngineState {
+            running: true,
+            pass_folder: Some(3),
+            pass_age_secs: SYNC_STALL_AFTER.as_secs() * 4,
+            pass_idle_secs: 5,
+            panics: 0,
+        };
+        assert!(!report_stalls(&with_sync(busy.clone()), (0, 0)));
+        let quiet = SyncEngineState {
+            pass_idle_secs: SYNC_STALL_AFTER.as_secs(),
+            ..busy
+        };
+        assert!(report_stalls(&with_sync(quiet), (0, 0)));
+        let idle = SyncEngineState {
+            running: true,
+            ..Default::default()
+        };
+        assert!(!report_stalls(&with_sync(idle), (0, 0)));
+    }
 }

crates/pdfs-fuse/src/sync.rs

@@ -49,7 +49,9 @@ mod local_scan;
 mod model;
 mod planner;
 pub(crate) use engine::SYNC_CONCURRENCY;
-pub(crate) use engine::{SyncMsg, spawn};
+pub(crate) use engine::{EngineHealth, SyncMsg, spawn};
+#[cfg(test)]
+use engine::{Running, burst_order, keep_wanted};
 #[cfg(test)]
 use engine::{classify, event_target, settle_with};
 #[cfg(test)]
@@ -67,6 +69,11 @@ use planner::{
     plan_file, rel_to_path, remote_sig, remote_subtree_changed, revision_moved, tracked_baseline,
     unchanged_remote_size,
 };
+
+/// A pass that took at least this long is logged at info even when it changed
+/// nothing, so the journal says where a slow engine spends its time.
+const SLOW_PASS: Duration = Duration::from_secs(30);
+
 /// What a [`Pending`] op did, so the reconcile can update shared state on the
 /// engine thread (never inside a task).
 enum Applied {
@@ -154,6 +161,14 @@ impl Core {
         // The cost is one small file read against a pass that is about to walk the
         // whole tree.
         let rules = self.ignore_rules(&local_root);
+        // A first pass is the one a user waits on after adding or restoring a
+        // folder, so it is said at info; the poll's passes stay at debug.
+        if folder.last_sync == 0 {
+            info!(id = folder.id, path = %folder.local_path, "sync: first pass starting");
+        } else {
+            debug!(id = folder.id, "sync: pass starting");
+        }
+        let started = Instant::now();
         self.progress_begin(folder.id);
         let result = if push_only {
             self.push_pass(folder.id, &local_root, &remote_root, &rules)
@@ -195,6 +210,18 @@ impl Core {
                     "idle"
                 };
                 let _ = self.db.sync_folder_set_state(folder.id, state, now_secs());
+                let elapsed = started.elapsed();
+                if folder.last_sync == 0 || !outcome.is_empty() || elapsed >= SLOW_PASS {
+                    info!(
+                        id = folder.id,
+                        state,
+                        elapsed_ms = elapsed.as_millis() as u64,
+                        summary = %outcome.summary(),
+                        "sync: pass finished"
+                    );
+                } else {
+                    debug!(id = folder.id, state, "sync: pass finished");
+                }
                 self.report_unscanned(folder.id, name, &outcome.unscanned);
                 // Summarise a pass that actually moved something. A poll over an
                 // unchanged folder does nothing and says nothing — otherwise the
@@ -2478,6 +2505,72 @@ fn system_mtime_ns(meta: &std::fs::Metadata) -> Option<i64> {
 mod tests {
     use super::*;
 
+    /// A panic in a pass ends that pass and is counted; the engine is not
+    /// left believing the pass still runs (`docs/BUGS.md` B259).
+    #[test]
+    fn a_panicking_pass_is_contained_and_counted() {
+        let health = EngineHealth::default();
+        assert!(!health.contain(7, || panic!("a pass that panics")));
+        let report = health.report();
+        assert_eq!(report.panics, 1);
+        assert_eq!(report.pass_folder, None);
+        assert!(health.contain(7, || {}));
+        assert_eq!(health.report().panics, 1);
+    }
+
+    /// Folders named by id run before the rest of a `ReconcileAll` in the
+    /// same burst, so a folder just added does not wait behind a pass over
+    /// every other folder (`docs/BUGS.md` B260).
+    #[test]
+    fn named_folders_run_before_the_rest_of_a_burst() {
+        let ids: HashSet<i64> = [9, 4].into();
+        assert_eq!(burst_order(&ids, &[1, 4, 6, 9]), [4, 9, 1, 6]);
+        assert_eq!(burst_order(&HashSet::new(), &[1, 4]), [1, 4]);
+        // A folder the list no longer has still gets the pass it asked for.
+        assert_eq!(burst_order(&[12].into(), &[1]), [12, 1]);
+    }
+
+    /// A folder removed and added again keeps its watch under the new id;
+    /// a folder that is gone loses its watch (`docs/BUGS.md` B272).
+    #[test]
+    fn a_re_added_folder_keeps_its_watch_under_the_new_id() {
+        let mut have = vec![(PathBuf::from("/a"), 1), (PathBuf::from("/b"), 2)];
+        let dropped = keep_wanted(&mut have, &[(PathBuf::from("/a"), 7)]);
+        assert_eq!(have, [(PathBuf::from("/a"), 7)]);
+        assert_eq!(dropped, [PathBuf::from("/b")]);
+    }
+
+    /// While a pass runs, the report names its folder, which is what tells a
+    /// long pass apart from an idle engine.
+    #[test]
+    fn the_running_pass_is_reported() {
+        let health = EngineHealth::default();
+        let mut seen = None;
+        health.contain(7, || seen = health.report().pass_folder);
+        assert_eq!(seen, Some(7));
+    }
+
+    /// The engine reads as running only while its thread lives, including
+    /// when the thread ends in a panic.
+    #[test]
+    fn the_engine_reads_as_stopped_once_its_thread_ends() {
+        let health = Arc::new(EngineHealth::default());
+        let thread_health = health.clone();
+        let (started_tx, started_rx) = std::sync::mpsc::channel();
+        let (end_tx, end_rx) = std::sync::mpsc::channel::<()>();
+        let thread = std::thread::spawn(move || {
+            let _running = Running::start(&thread_health);
+            started_tx.send(()).unwrap();
+            let _ = end_rx.recv();
+            panic!("the engine thread panics");
+        });
+        started_rx.recv().unwrap();
+        assert!(health.report().running);
+        end_tx.send(()).unwrap();
+        assert!(thread.join().is_err());
+        assert!(!health.report().running);
+    }
+
     /// A removal of something already gone has produced the state the sync
     /// engine wanted, so its baseline is dropped rather than retried forever.
     /// Anything else stays an error and keeps the baseline (`docs/BUGS.md` B42).

crates/pdfs-fuse/src/sync/engine.rs

@@ -1,5 +1,11 @@
 //! Filesystem watcher, polling, and debounce control plane.
 
+use std::panic::AssertUnwindSafe;
+use std::sync::atomic::{AtomicBool, AtomicU64, Ordering};
+
+use pdfs_core::control::SyncEngineState;
+use tracing::error;
+
 use super::*;
 
 /// How long the watcher must go quiet before a burst counts as finished.
@@ -23,6 +29,9 @@ const POLL_INTERVAL: Duration = Duration::from_secs(120);
 /// unusable done one-at-a-time; this bounds concurrency like the bulk-upload
 /// engine does.
 pub(crate) const SYNC_CONCURRENCY: usize = 8;
+/// How long after a failed watch the engine tries the watches again. Each try
+/// walks the folder's whole tree, so not on every burst.
+const REWATCH_RETRY: Duration = Duration::from_secs(600);
 
 /// A message to the sync engine's control thread.
 pub(crate) enum SyncMsg {
@@ -53,32 +62,159 @@ pub(crate) fn spawn(core: Core, rx: Receiver<SyncMsg>) -> Option<std::thread::Jo
     }
 }
 
+/// Whether the engine is alive and which pass it is on, for
+/// [`Core::diagnostics`] and the supervisor.
+///
+/// A panic in a pass used to end the engine thread. Every later add, sync-now,
+/// file event and poll then went into a channel nobody read, the folder stayed
+/// `syncing`, and nothing said so until a restart (`docs/BUGS.md` B259).
+#[derive(Default)]
+pub(crate) struct EngineHealth {
+    running: AtomicBool,
+    pass: Mutex<Option<PassClock>>,
+    panics: AtomicU64,
+}
+
+#[derive(Clone, Copy)]
+struct PassClock {
+    folder: i64,
+    started: Instant,
+    moved: Instant,
+}
+
+impl EngineHealth {
+    /// Note that the running pass reported progress.
+    pub(crate) fn moved(&self) {
+        if let Some(pass) = self.pass.lock().as_mut() {
+            pass.moved = Instant::now();
+        }
+    }
+
+    /// The engine's state now. `try_lock`, like the rest of the diagnostics:
+    /// a report must not wait on the thing it reports about.
+    pub(crate) fn report(&self) -> SyncEngineState {
+        let pass = self.pass.try_lock().and_then(|pass| *pass);
+        SyncEngineState {
+            running: self.running.load(Ordering::Relaxed),
+            pass_folder: pass.map(|p| p.folder),
+            pass_age_secs: pass.map_or(0, |p| p.started.elapsed().as_secs()),
+            pass_idle_secs: pass.map_or(0, |p| p.moved.elapsed().as_secs()),
+            panics: self.panics.load(Ordering::Relaxed),
+        }
+    }
+
+    /// Run `pass` over `folder` as the current pass. Returns `false` when it
+    /// panicked: the panic ends that pass, not the engine.
+    pub(super) fn contain(&self, folder: i64, pass: impl FnOnce()) -> bool {
+        let now = Instant::now();
+        *self.pass.lock() = Some(PassClock {
+            folder,
+            started: now,
+            moved: now,
+        });
+        let finished = std::panic::catch_unwind(AssertUnwindSafe(pass)).is_ok();
+        *self.pass.lock() = None;
+        if !finished {
+            self.panics.fetch_add(1, Ordering::Relaxed);
+        }
+        finished
+    }
+}
+
+/// Marks the engine running for as long as it lives.
+pub(super) struct Running<'a>(&'a EngineHealth);
+
+impl<'a> Running<'a> {
+    pub(super) fn start(health: &'a EngineHealth) -> Self {
+        health.running.store(true, Ordering::Relaxed);
+        Self(health)
+    }
+}
+
+impl Drop for Running<'_> {
+    fn drop(&mut self) {
+        self.0.running.store(false, Ordering::Relaxed);
+        if std::thread::panicking() {
+            error!("sync engine stopped by a panic; nothing syncs until the daemon restarts");
+        }
+    }
+}
+
+/// One folder's pass, contained: a panic in it sets the folder to `error` and
+/// leaves the engine running for the next one.
+fn run_pass(core: &Core, folder: &StoredSyncFolder) {
+    if core
+        .sync_engine
+        .contain(folder.id, || core.reconcile_folder(folder))
+    {
+        return;
+    }
+    core.progress_end(folder.id);
+    let _ = core
+        .db
+        .sync_folder_set_state(folder.id, "error", folder.last_sync);
+    error!(
+        id = folder.id,
+        path = %folder.local_path,
+        "sync: pass panicked; the folder is set to error and tried again on the next poll"
+    );
+    core.log_activity(
+        ActivityKind::Sync,
+        format!("sync failed for {}", base_name(&folder.local_path)),
+        "an internal error stopped the pass; it is tried again on the next poll",
+        false,
+    );
+}
+
+/// The pass a `Reconcile(id)` asked for. A folder removed since, or one the
+/// database cannot read, is said so rather than dropped in silence.
+fn run_pass_by_id(core: &Core, id: i64) {
+    match core.db.sync_folder_get(id) {
+        Ok(Some(folder)) => run_pass(core, &folder),
+        Ok(None) => debug!(id, "sync: folder removed before its pass; skipping"),
+        Err(e) => warn!(id, error = ?e, "sync: cannot read folder; skipping its pass"),
+    }
+}
+
 /// The engine control loop. Single-threaded so reconcile passes never overlap.
 fn engine_loop(core: Core, rx: Receiver<SyncMsg>) {
+    let health = core.sync_engine.clone();
+    let _running = Running::start(&health);
     // Paths the watcher currently covers, mapped to their folder id, so an event
     // path can be resolved back to the folder it belongs to.
     let watched: Arc<Mutex<Vec<(PathBuf, i64)>>> = Arc::new(Mutex::new(Vec::new()));
 
     let tx_events = core.sync_tx.clone();
     let watched_events = watched.clone();
-    let mut watcher =
-        match notify::recommended_watcher(move |res: notify::Result<notify::Event>| {
-            let Ok(event) = res else { return };
-            // Ignore pure metadata/access noise; content changes are what matter.
-            if matches!(event.kind, notify::EventKind::Access(_)) {
-                return;
-            }
-            let msg = event_target(&watched_events.lock(), &event.paths);
-            let _ = tx_events.send(msg);
-        }) {
-            Ok(w) => w,
+    let handler = move |res: notify::Result<notify::Event>| {
+        let event = match res {
+            Ok(event) => event,
+            // Running out of inotify watches for a new subfolder, say: that
+            // subfolder's changes are then found by the poll only (B262).
             Err(e) => {
-                warn!(error = %e, "filesystem watcher unavailable; sync is poll-only");
-                // Still run the loop so polling and explicit SyncNow work.
-                poll_only_loop(&core, rx);
+                warn!(error = %e, "sync: filesystem watcher error");
                 return;
             }
         };
+        // Ignore pure metadata/access noise; content changes are what matter.
+        if matches!(event.kind, notify::EventKind::Access(_)) {
+            return;
+        }
+        let msg = event_target(&watched_events.lock(), &event.paths);
+        let _ = tx_events.send(msg);
+    };
+    // The local walk skips symlinks, so following them here would only watch
+    // trees outside the folder, and run passes over it for changes made there.
+    let config = notify::Config::default().with_follow_symlinks(false);
+    let mut watcher = match notify::RecommendedWatcher::new(handler, config) {
+        Ok(w) => w,
+        Err(e) => {
+            warn!(error = %e, "filesystem watcher unavailable; sync is poll-only");
+            // Still run the loop so polling and explicit SyncNow work.
+            poll_only_loop(&core, rx);
+            return;
+        }
+    };
 
     // Periodic remote poll.
     {
@@ -96,8 +232,9 @@ fn engine_loop(core: Core, rx: Receiver<SyncMsg>) {
             .ok();
     }
 
-    rewatch(&core, &mut watcher, &watched);
-    reconcile_all(&core);
+    let mut unwatched = rewatch(&core, &mut watcher, &watched);
+    let mut last_rewatch = Instant::now();
+    run_burst(&core, &HashSet::new(), true);
 
     while let Ok(msg) = rx.recv() {
         let mut ids: HashSet<i64> = HashSet::new();
@@ -110,21 +247,16 @@ fn engine_loop(core: Core, rx: Receiver<SyncMsg>) {
             return;
         }
 
+        // A folder whose watch failed is tried again now and then, not only
+        // when the folder set changes.
+        if unwatched && last_rewatch.elapsed() >= REWATCH_RETRY {
+            do_rewatch = true;
+        }
         if do_rewatch {
-            rewatch(&core, &mut watcher, &watched);
-        }
-        if all {
-            reconcile_all(&core);
-        } else {
-            for id in ids {
-                if core.sync_paused() {
-                    break;
-                }
-                if let Ok(Some(folder)) = core.db.sync_folder_get(id) {
-                    core.reconcile_folder(&folder);
-                }
-            }
+            unwatched = rewatch(&core, &mut watcher, &watched);
+            last_rewatch = Instant::now();
         }
+        run_burst(&core, &ids, all);
     }
 }
 
@@ -141,7 +273,7 @@ fn poll_only_loop(core: &Core, rx: Receiver<SyncMsg>) {
             }
         });
     }
-    reconcile_all(core);
+    run_burst(core, &HashSet::new(), true);
     while let Ok(msg) = rx.recv() {
         let mut ids: HashSet<i64> = HashSet::new();
         let mut all = false;
@@ -152,21 +284,59 @@ fn poll_only_loop(core: &Core, rx: Receiver<SyncMsg>) {
             info!("sync engine stopping");
             return;
         }
-        if all {
-            reconcile_all(core);
-        } else {
-            for id in ids {
-                if core.sync_paused() {
-                    break;
-                }
-                if let Ok(Some(folder)) = core.db.sync_folder_get(id) {
-                    core.reconcile_folder(&folder);
-                }
+        run_burst(core, &ids, all);
+    }
+}
+
+/// Run the passes one burst asked for: the folders named by id, then, when the
+/// burst also holds a `ReconcileAll`, every other folder.
+///
+/// The named folders go first. A poll's `ReconcileAll` in the same burst used
+/// to take their place, and a folder just added, which has the highest id,
+/// waited behind a full pass over every other folder (`docs/BUGS.md` B260).
+///
+/// Skipped while sync is paused, and cut short when a pause lands between
+/// passes: the resume sends a fresh [`SyncMsg::ReconcileAll`], which walks
+/// everything the watcher reported meanwhile.
+fn run_burst(core: &Core, ids: &HashSet<i64>, all: bool) {
+    if core.sync_paused() {
+        debug!("sync: paused, skipping reconcile");
+        return;
+    }
+    let folders = if all {
+        match core.db.sync_folder_list() {
+            Ok(folders) => folders,
+            Err(e) => {
+                warn!(error = ?e, "sync: cannot list folders");
+                Vec::new()
             }
         }
+    } else {
+        Vec::new()
+    };
+    let listed: Vec<i64> = folders.iter().map(|f| f.id).collect();
+    for id in burst_order(ids, &listed) {
+        if core.sync_paused() {
+            break;
+        }
+        // A folder named by id is read again: it may have been added or
+        // changed after the list was read.
+        match folders.iter().find(|f| f.id == id) {
+            Some(folder) if !ids.contains(&id) => run_pass(core, folder),
+            _ => run_pass_by_id(core, id),
+        }
     }
 }
 
+/// The order of a burst's passes: the folders named by id, lowest first, then
+/// the rest of `listed` in its own order.
+pub(super) fn burst_order(ids: &HashSet<i64>, listed: &[i64]) -> Vec<i64> {
+    let mut order: Vec<i64> = ids.iter().copied().collect();
+    order.sort_unstable();
+    order.extend(listed.iter().copied().filter(|id| !ids.contains(id)));
+    order
+}
+
 /// Absorb the rest of an event burst, returning once the watcher has been quiet
 /// for `quiet` or `cap` has elapsed since the first event — whichever comes
 /// first. The caller has already classified the event that opened the burst.
@@ -238,26 +408,6 @@ pub(super) fn classify(
     true
 }
 
-/// Reconcile every mirror folder in turn. Skipped while sync is paused: the
-/// resume sends a fresh [`SyncMsg::ReconcileAll`], which walks everything the
-/// watcher reported meanwhile.
-fn reconcile_all(core: &Core) {
-    if core.sync_paused() {
-        debug!("sync: paused, skipping reconcile");
-        return;
-    }
-    let folders = match core.db.sync_folder_list() {
-        Ok(f) => f,
-        Err(e) => {
-            warn!(error = ?e, "sync: cannot list folders");
-            return;
-        }
-    };
-    for folder in folders {
-        core.reconcile_folder(&folder);
-    }
-}
-
 /// The reconcile a watcher event asks for: the one mirror folder all its paths
 /// lie in, so an edit in one folder does not walk every other one too. An event
 /// that spans folders, or that no watched folder claims (a watch list a moment
@@ -278,7 +428,8 @@ pub(super) fn event_target(watched: &[(PathBuf, i64)], paths: &[PathBuf]) -> Syn
 }
 
 /// Bring the filesystem watches in line with the current mirror-folder set:
-/// watch newly-added folders, drop removed ones.
+/// watch newly-added folders, drop removed ones. Returns whether a folder is
+/// left unwatched because its watch failed.
 ///
 /// The watch calls run on a copy, outside the lock: the watcher's event
 /// callback takes the same lock, and a backend that hands a watch request to
@@ -287,7 +438,7 @@ fn rewatch(
     core: &Core,
     watcher: &mut notify::RecommendedWatcher,
     watched: &Mutex<Vec<(PathBuf, i64)>>,
-) {
+) -> bool {
     let want: Vec<(PathBuf, i64)> = match core.db.sync_folder_list() {
         Ok(f) => f
             .into_iter()
@@ -296,30 +447,60 @@ fn rewatch(
             .collect(),
         Err(e) => {
             warn!(error = ?e, "sync: cannot list folders to watch");
-            return;
+            return false;
         }
     };
     let mut have = watched.lock().clone();
     // Drop watches no longer wanted.
-    have.retain(|(path, _)| {
-        if want.iter().any(|(p, _)| p == path) {
-            true
-        } else {
-            let _ = watcher.unwatch(path);
-            false
-        }
-    });
+    for path in keep_wanted(&mut have, &want) {
+        let _ = watcher.unwatch(&path);
+    }
     // Add newly wanted watches.
+    let mut failed = false;
     for (path, id) in &want {
         if have.iter().any(|(p, _)| p == path) {
             continue;
         }
         match watcher.watch(path, RecursiveMode::Recursive) {
             Ok(()) => have.push((path.clone(), *id)),
-            Err(e) => warn!(path = %path.display(), error = %e, "sync: watch failed"),
+            Err(e) => {
+                // A recursive watch that fails partway keeps the watches it had
+                // added. Their events would match no folder and run a pass over
+                // every one, and they hold watches the next folder needs (B262).
+                let _ = watcher.unwatch(path);
+                failed = true;
+                warn!(
+                    path = %path.display(),
+                    error = %e,
+                    "sync: watch failed; the poll finds this folder's changes until a retry \
+                     works. A low fs.inotify.max_user_watches is the usual cause"
+                );
+            }
         }
     }
     *watched.lock() = have;
+    failed
+}
+
+/// Keep the watches in `have` that `want` still asks for, and return the paths
+/// of the others, to unwatch.
+///
+/// A path is matched alone, but takes the id `want` gives it: a folder removed
+/// and added again in one burst keeps its watch, and its events go to the new
+/// row rather than to the removed one (`docs/BUGS.md` B272).
+pub(super) fn keep_wanted(have: &mut Vec<(PathBuf, i64)>, want: &[(PathBuf, i64)]) -> Vec<PathBuf> {
+    let mut dropped = Vec::new();
+    have.retain_mut(|(path, id)| match want.iter().find(|(p, _)| p == path) {
+        Some((_, wanted)) => {
+            *id = *wanted;
+            true
+        }
+        None => {
+            dropped.push(path.clone());
+            false
+        }
+    });
+    dropped
 }
 
 // ---- reconcile ------------------------------------------------------------

crates/pdfs-fuse/src/systemd.rs

@@ -64,11 +64,19 @@ fn send_abstract(socket: &UnixDatagram, name: &str, message: &str) -> bool {
 /// so `systemctl --user start` returns when the mount actually works rather than
 /// when the process exists.
 pub(crate) fn ready() {
-    if notify("READY=1\n") {
+    if notify("READY=1\nSTATUS=Mounted\n") {
         debug!("notified systemd: ready");
     }
 }
 
+/// Set the one-line status `systemctl --user status` shows for the unit, such
+/// as why the mount keeps failing (`docs/BUGS.md` B263).
+pub fn status(text: &str) {
+    // A newline would end the `STATUS=` line and start another variable.
+    let line = text.replace('\n', " ");
+    notify(&format!("STATUS={line}\n"));
+}
+
 /// Tell the manager the daemon is still healthy.
 pub(crate) fn watchdog_ping() {
     let _ = notify("WATCHDOG=1\n");