From 637b512f6a2209be181f8149085f35469e90d596 Mon Sep 17 00:00:00 2001 From: Kacper Paczos <23406555+kacperpaczos@users.noreply.github.com> Date: Tue, 25 Aug 2026 13:24:46 +0200 Subject: [PATCH 1/2] fix(cli): back off while waiting for a stored session The daemon's wait-for-login loop slept a flat 3 seconds between checks and never gave up, so an enabled service with no saved session polled the keyring 28,800 times a day. Each check is more than a sleep: auth::load() builds a fresh keyring::Entry, so every iteration opens a new D-Bus connection to the Secret Service and negotiates a session on it. Back off from 3s to a 60s ceiling and say "not logged in" once at INFO rather than on every iteration, which also stops the journal filling with tens of thousands of identical lines. The wait is extracted into wait_for_session() taking the load and sleep calls as parameters, so the schedule is testable without a keyring and without real time passing. Refs #21 --- crates/pdfs-cli/src/main.rs | 120 +++++++++++++++++++++++++++++++++--- 1 file changed, 110 insertions(+), 10 deletions(-) diff --git a/crates/pdfs-cli/src/main.rs b/crates/pdfs-cli/src/main.rs index 9e6029a..78851b9 100644 --- a/crates/pdfs-cli/src/main.rs +++ b/crates/pdfs-cli/src/main.rs @@ -1448,6 +1448,54 @@ 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 60s 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 roughly 48,000 of those a +/// day from a daemon that had nothing to do. +fn login_poll_delay(attempts: u32) -> std::time::Duration { + const BASE_SECS: u64 = 3; + const MAX_SECS: u64 = 60; + + // `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 @@ -1456,16 +1504,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) => { @@ -2537,3 +2576,64 @@ 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(24)); + assert_eq!(login_poll_delay(4), Duration::from_secs(48)); + assert_eq!(login_poll_delay(5), Duration::from_secs(60)); + // 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(60)); + assert_eq!(login_poll_delay(u32::MAX), Duration::from_secs(60)); + } + + #[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(15 * 60), + "20 checks should span at least 15 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); + } +} From 45852082c0380102affaf9e1cecb5086e7975859 Mon Sep 17 00:00:00 2001 From: Nils Pukropp Date: Tue, 1 Sep 2026 16:28:25 +0200 Subject: [PATCH 2/2] fix(cli,core): wake the parked login-wait daemon on sign-in The login backoff had no wake-up path. A unit started at boot sits in wait_for_session and reaches the ceiling within ~90s; the user then signs in and `enable --now` is a no-op on an already-active unit, so the daemon kept sleeping and the mount appeared up to a full interval late. - service::enable_start now runs `enable` + `restart` instead of `enable --now`, so the GUI login and the tray's Connect interrupt the sleep immediately and still start a unit that is down. - `pdfs login` restarts the unit when it is already active. It stays out of `enable` on purpose, so a headless setup driving `pdfs daemon` by hand is left alone. - Lower the backoff ceiling from 60s to 15s. It is still an 80% cut in keyring/D-Bus wakeups (28,800/day -> 5,760/day) and it bounds the worst-case latency for paths that never touch systemd. Also corrects the flat-3s-poll figure in the doc comment: 86400/3 is 28,800 checks a day, not 48,000. --- crates/pdfs-cli/src/main.rs | 36 +++++++++++++++++++++++---------- crates/pdfs-core/src/service.rs | 14 ++++++++++--- 2 files changed, 36 insertions(+), 14 deletions(-) diff --git a/crates/pdfs-cli/src/main.rs b/crates/pdfs-cli/src/main.rs index 78851b9..e79f176 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( @@ -1309,6 +1310,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(()) } @@ -1451,14 +1461,19 @@ fn cmd_mount(mountpoint: Option) -> Result<()> { /// 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 60s ceiling. The interval is not just a sleep: +/// 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 roughly 48,000 of those a -/// day from a daemon that had nothing to do. +/// 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 = 60; + 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. @@ -2587,13 +2602,12 @@ mod login_wait_tests { 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(24)); - assert_eq!(login_poll_delay(4), Duration::from_secs(48)); - assert_eq!(login_poll_delay(5), Duration::from_secs(60)); + 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(60)); - assert_eq!(login_poll_delay(u32::MAX), Duration::from_secs(60)); + assert_eq!(login_poll_delay(64), Duration::from_secs(15)); + assert_eq!(login_poll_delay(u32::MAX), Duration::from_secs(15)); } #[test] @@ -2625,8 +2639,8 @@ mod login_wait_tests { // the way a flat 3s poll did. let total: Duration = slept.iter().sum(); assert!( - total >= Duration::from_secs(15 * 60), - "20 checks should span at least 15 minutes, spanned {total:?}" + total >= Duration::from_secs(4 * 60), + "20 checks should span at least 4 minutes, spanned {total:?}" ); } 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