fix(cli): back off while waiting for a stored session - #22
Conversation
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 narrrl#21
| /// 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; |
There was a problem hiding this comment.
The 60s ceiling has no wake-up path, so it turns into up to a minute of dead time between logging in and having a mount.
Concrete scenario: the unit is enabled, so it starts at boot and sits in wait_for_session, reaching the ceiling within ~90s. The user then logs in through the GUI, which calls service::enable_start() -> systemctl --user enable --now proton-drive.service (crates/pdfs-core/src/service.rs:51). On an already-active unit --now is a no-op, so nothing interrupts the sleeping daemon; it keeps sleeping for up to the full 60s before it notices the freshly stored session. The tray's "Connect" item (crates/pdfs-gui/src/main.rs:231) has the same problem — it also just calls enable_start(), so the user gets "Connecting..." for up to a minute with nothing happening. pdfs login on the CLI never touches systemd at all, so it is the same wait there. Previously the worst case was 3s.
The polling cost is real and worth fixing, but the fix needs a way to shorten the wait when a session actually appears. Cheapest options: cap the ceiling much lower (10-15s still removes ~90% of the D-Bus sessions), or have the login paths call service::restart() instead of enable_start() when service::is_active() is already true, so the waiting daemon is torn down and comes back at attempt 0.
| /// 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 |
There was a problem hiding this comment.
Nit, but it is the number a future reader will trust: a flat 3s poll is 86400/3 = 28,800 checks a day, not 48,000 (the PR description says 28,800 too).
| /// encrypted session over it. A flat 3s poll meant roughly 48,000 of those a | |
| /// encrypted session over it. A flat 3s poll meant roughly 28,800 of those a |
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.
What this changes
An enabled
proton-drive.servicewith no saved session used to poll the keyringevery 3 seconds forever. Now the interval backs off to a 60-second ceiling, and
the "not logged in" line is logged once instead of on every iteration.
It matters more than a sleep does, because each check is a fresh D-Bus session:
auth::load()builds a newkeyring::Entryevery call, so every poll opens a newconnection to the Secret Service and negotiates on it. Measured on Fedora 44 with
nothing else running, the daemon made one
OpenSessionper poll — ~28,800 a daydoing nothing. One 24-hour boot on my machine logged 23,928 of those lines across
four daemon instances.
Crates touched:
pdfs-cliRelated
docs/BUGS.mdentries: none — I didn't want to allocate a B-number inyour ledger from outside. Reported as #21; happy to add an entry if you'd like one.
Verification
One gap, stated plainly: I could not run the
--workspaceforms of clippy andtest, because
pdfs-guineeds WebKitGTK 6 development headers that aren'tavailable on this machine. Everything the change touches is covered;
pdfs-guiisuntouched by it, but I haven't compiled that crate and won't claim I have.
The three new tests fail without the fix — I reverted the backoff to a flat 3s and
confirmed red before restoring it:
I also ran the built binary against a real Secret Service with no session stored.
Before: 20 polls a minute. After:
delay_secs=6, 12, 24, 48, five polls in thefirst minute and one a minute thereafter.
Implications
Not covered here
pdfs-trayhas the same pattern independently —POLL_INTERVALis 3s(
crates/pdfs-gui/src/main.rs:27) and when the daemon socket doesn't answer itfalls through to
auth::load()(main.rs:121), so it keeps the flat 3-secondkeyring poll going on its own. I left it out because I can't build or test that
crate here, and an untested change to it would just be a guess. Details in
#21.
Say the word and I'll do it in a follow-up once I can compile it, or you may
prefer both to share one helper.