diff --git a/crates/pdfs-cli/src/main.rs b/crates/pdfs-cli/src/main.rs index 86d6e70..3ac39f6 100644 --- a/crates/pdfs-cli/src/main.rs +++ b/crates/pdfs-cli/src/main.rs @@ -17,6 +17,7 @@ use pdfs_core::control::{ RestoreItem as CtlRestoreItem, ShareEntryKind, SyncPhase, pending_summary, }; use pdfs_core::db::Db; +use pdfs_core::service; #[derive(Parser)] #[command( @@ -1355,6 +1356,15 @@ fn cmd_login(username: Option) -> Result<()> { other => anyhow::Error::new(other).context("login failed"), })?; + // A daemon that is already up is parked in `wait_for_session`, sleeping + // between keyring checks; restart it so the mount appears now instead of + // whenever the next poll fires. Only when it is already active — `pdfs + // login` deliberately does not enable the unit the way the GUI does, so a + // headless user driving `pdfs daemon` by hand keeps that setup. + if service::is_active() { + service::restart(); + } + println!("Logged in as {username}. Session stored in the system keyring."); Ok(()) } @@ -1494,6 +1504,59 @@ fn cmd_mount(mountpoint: Option) -> Result<()> { Ok(()) } +/// How long to wait before the next login check, after `attempts` checks have +/// already come back [`pdfs_core::Error::NotLoggedIn`]. +/// +/// Doubles from 3s to a 15s ceiling. The interval is not just a sleep: +/// [`auth::load`] builds a fresh `keyring::Entry` on every call, so each check +/// opens a new D-Bus connection to the Secret Service and negotiates an +/// encrypted session over it. A flat 3s poll meant 28,800 of those a day from a +/// daemon that had nothing to do; the ceiling cuts that by 80%. +/// +/// The ceiling stays low because the sleep is also the worst-case login → +/// mount latency for anything that does *not* poke systemd: front-ends restart +/// the unit on login (`service::enable_start`), which interrupts the sleep +/// immediately, but a hand-run `pdfs daemon` has no such wake-up path. +fn login_poll_delay(attempts: u32) -> std::time::Duration { + const BASE_SECS: u64 = 3; + const MAX_SECS: u64 = 15; + + // `checked_shl` rather than `<<`: the daemon can sit unauthenticated for + // weeks, and a shift past the width of the type is a panic, not a big number. + let factor = 1u64.checked_shl(attempts).unwrap_or(u64::MAX); + std::time::Duration::from_secs(BASE_SECS.saturating_mul(factor).min(MAX_SECS)) +} + +/// Block until a session is stored, backing off between checks. +/// +/// `load` and `sleep` are parameters rather than direct calls so the backoff +/// schedule can be tested without a keyring and without real time passing. +fn wait_for_session(mut load: L, mut sleep: S) -> Result<()> +where + L: FnMut() -> std::result::Result<(), pdfs_core::Error>, + S: FnMut(std::time::Duration), +{ + let mut attempts: u32 = 0; + loop { + match load() { + Ok(()) => return Ok(()), + Err(pdfs_core::Error::NotLoggedIn) => { + let delay = login_poll_delay(attempts); + // Say it once at INFO. Repeating it every interval buried the + // journal in tens of thousands of identical lines a day. + if attempts == 0 { + tracing::info!("not logged in; waiting for a session…"); + } else { + tracing::debug!(delay_secs = delay.as_secs(), "still not logged in"); + } + sleep(delay); + attempts = attempts.saturating_add(1); + } + Err(e) => return Err(e.into()), + } + } +} + /// Run the auto-mount daemon loop. Waits for a stored login, mounts, and keeps /// the mount alive: a clean stop (SIGTERM via `systemctl --user stop`) exits 0; /// an external unmount triggers a remount; errors back off and retry. This is @@ -1502,16 +1565,7 @@ fn cmd_daemon(mountpoint: Option) -> Result<()> { loop { // Wait until a session is stored. The GUI enables this service on login, // but the service may also start at boot before the user has logged in. - loop { - match auth::load() { - Ok(_) => break, - Err(pdfs_core::Error::NotLoggedIn) => { - tracing::info!("not logged in; waiting…"); - std::thread::sleep(std::time::Duration::from_secs(3)); - } - Err(e) => return Err(e.into()), - } - } + wait_for_session(|| auth::load().map(|_| ()), std::thread::sleep)?; match mount_once(mountpoint.clone()) { Ok(pdfs_fuse::MountOutcome::Shutdown) => { @@ -2692,3 +2746,63 @@ mod diagnose_tests { std::fs::remove_dir(dir).unwrap(); } } + +#[cfg(test)] +mod login_wait_tests { + use super::*; + use std::time::Duration; + + #[test] + fn poll_delay_doubles_then_holds_at_the_ceiling() { + assert_eq!(login_poll_delay(0), Duration::from_secs(3)); + assert_eq!(login_poll_delay(1), Duration::from_secs(6)); + assert_eq!(login_poll_delay(2), Duration::from_secs(12)); + assert_eq!(login_poll_delay(3), Duration::from_secs(15)); + assert_eq!(login_poll_delay(4), Duration::from_secs(15)); + // A daemon left unauthenticated for weeks reaches attempt counts well + // past the width of the shift; that must cap, not panic. + assert_eq!(login_poll_delay(64), Duration::from_secs(15)); + assert_eq!(login_poll_delay(u32::MAX), Duration::from_secs(15)); + } + + #[test] + fn waiting_for_a_session_stops_hammering_the_keyring() { + let mut checks_left = 20; + let mut slept: Vec = Vec::new(); + + wait_for_session( + || { + if checks_left == 0 { + Ok(()) + } else { + checks_left -= 1; + Err(pdfs_core::Error::NotLoggedIn) + } + }, + |delay| slept.push(delay), + ) + .unwrap(); + + assert_eq!(slept.len(), 20, "one wait per failed check"); + assert!( + slept.windows(2).all(|w| w[1] >= w[0]), + "the interval must never shrink: {slept:?}" + ); + + // The point of the change. Every check is a fresh D-Bus session against + // the Secret Service, so 20 of them must not fit inside a single minute + // the way a flat 3s poll did. + let total: Duration = slept.iter().sum(); + assert!( + total >= Duration::from_secs(4 * 60), + "20 checks should span at least 4 minutes, spanned {total:?}" + ); + } + + #[test] + fn a_stored_session_is_not_waited_for_at_all() { + let mut slept = 0; + wait_for_session(|| Ok(()), |_| slept += 1).unwrap(); + assert_eq!(slept, 0); + } +} diff --git a/crates/pdfs-core/src/service.rs b/crates/pdfs-core/src/service.rs index b0855c5..745e07f 100644 --- a/crates/pdfs-core/src/service.rs +++ b/crates/pdfs-core/src/service.rs @@ -47,10 +47,18 @@ fn systemctl_query(args: &[&str]) -> bool { .unwrap_or(false) } -/// Enable the service so it survives reboots/logins and start it now. Called on -/// successful login. +/// Enable the service so it survives reboots/logins, and bring it up now. +/// Called on successful login. +/// +/// `restart` rather than `enable --now`: the unit may already be active but +/// parked in the daemon's login-wait backoff (`pdfs daemon` sleeps between +/// keyring checks). `--now` is a no-op on an active unit, so the daemon would +/// keep sleeping and the mount would not appear until the next poll fired. +/// Restarting makes the new session take effect immediately, and starts the +/// unit when it is not running at all. pub fn enable_start() -> bool { - systemctl(&["enable", "--now"]) + let enabled = systemctl(&["enable"]); + systemctl(&["restart"]) && enabled } /// Stop the running mount without disabling it. Called by the tray's