fix: keep the daemon's timers on a monotonic clock - #516
Merged
Merged
Conversation
lyager
marked this pull request as draft
October 1, 2026 09:02
Every deadline the daemon keeps — record TTLs and refresh points, probe and announce schedules, retransmissions, delayed responses, resolver timeouts and the periodic IP check — was a millisecond value derived from `SystemTime`. A step of the system clock therefore moved all of them: a backwards step parked them until the wall clock had caught up, so a daemon that had registered its services just before the step never announced them and never noticed a new address; a forwards step fired them all at once and made every cached record look expired. Seen on an embedded device whose RTC driver loads a few seconds after the responder starts. The kernel re-seats the wall clock from the RTC at that point, and with an RTC holding a date in the past the responder went silent on every interface for the rest of the uptime, while `register()` had already reported success. Use `std::time::Instant` for points in time and `Duration` for the deltas between them. `current_time_millis()` is removed; the fields and parameters that carried its value are `Instant` now, and the `0` sentinels that meant "no time" are `Option<Instant>`. Differences use the standard saturating operations, so the two places that could underflow when a record was already past its expiry are covered as well. TTLs on the wire are still remaining seconds computed from the record's creation, so packets are unchanged. Nothing in the crate needs wall-clock time. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
lyager
marked this pull request as ready for review
October 1, 2026 09:29
lyager
added a commit
to pascal-audio/mdns-sd
that referenced
this pull request
Oct 2, 2026
…kport) `current_time_millis()` followed `SystemTime`, and every timer the daemon keeps — the periodic IP check, probing and announce deadlines, record TTLs, retransmissions — is a millisecond value from it. A backwards step of the system clock therefore moved all of them into the future for as long as the step was, and a daemon that had registered its services just before the step never announced them and never noticed a new address. A forwards step had the mirror effect: every timer fired at once and every cached record looked expired. Seen on PX amplifiers whose RTC driver loads a few seconds after thrust starts: the kernel re-seats the wall clock from the RTC, and when that RTC held a date in the past the responder went silent on every interface for the rest of the uptime (Pascal TH-1185). Anchor the value to the wall clock once, on first use, and advance it with `Instant` from then on. It keeps the magnitude of a UNIX timestamp, so nothing that compares or prints it changes, but it can no longer jump. `dns_parser` had a private copy of the helper; it now uses the shared one. This is the minimal backport for the January base that shipped firmware pins. The `pascal/v0.21.4` line carries the full `Instant`/`Duration` typing of the same fix (upstream PR keepsimple1#516). Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
keepsimple1
approved these changes
Oct 3, 2026
keepsimple1
left a comment
Owner
There was a problem hiding this comment.
Thank you for the PR! It's a real improvement, looks good!
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
Every deadline the daemon keeps is derived from
current_time_millis(), which readsSystemTime: record TTLs and refresh points, probe and announce schedules, retransmissions, delayed responses, resolver timeouts and the periodic IP check. If the system clock is stepped while the daemon runs, all of them move with it.A backwards step parks every deadline for the size of the step: the IP check stops running, a service registered just before the step is never announced, new addresses are never picked up, and
register()has already reported success. A forwards step has the mirror effect: every timer fires at once and every cached record looks expired.We hit the backwards case on an embedded device whose RTC driver loads a few seconds after the responder starts. The kernel re-seats the wall clock from the RTC at that point, and when the RTC held a date in the past the responder went silent on every interface for the rest of the uptime, with no error logged anywhere. Any host where NTP or an operator sets the clock back is exposed the same way.
Change
Use
std::time::Instantfor points in time andDurationfor the deltas between them, which is what the standard library provides for exactly this distinction.current_time_millis()is removed. Fields and parameters that carried its value (created,expires,refresh, probestart_time/next_send,ReRun/DelayedResponse::next_time, resolver timeouts, the timer heap) areInstant.0sentinels meaning "no time" (write_record,add_answer_at_time, the answers' per-record time, the IP-check timer) becomeOption<Instant>, since anInstanthas no zero.saturating_duration_since), so the two subtractions that could underflow when a record was already past its expiry are covered too. The run loop's poll timeout istimer.saturating_duration_since(now).max(1 ms).u64constant and is compared as aDurationat its single use site.const fns becomefn, becauseInstantcomparison and arithmetic go through traits. None was used in a const context.Nothing in the crate needs the wall-clock time; no absolute timestamp leaves the crate.
Testing
cargo testpasses (lib, integration, doc);cargo clippy --all-targetsandcargo fmt --checkare clean.Notes
InstantisCLOCK_MONOTONICon Linux and does not advance during suspend, so a cache on a machine that sleeps will trust records for longer than their TTL after waking. TheSystemTimeversion had the opposite failure on wake (everything expires at once). A re-query on resume is the usual answer if that matters to a user.current_time_millis()and anchor it to anInstant— which fixes the stall in one function but keeps milliseconds as untyped integers. I'm happy to switch to that if you prefer the minimal diff.