Skip to content

fix: keep the daemon's timers on a monotonic clock - #516

Merged
keepsimple1 merged 1 commit into
keepsimple1:mainfrom
pascal-audio:instant-timers
Oct 3, 2026
Merged

keepsimple1 merged 1 commit into
keepsimple1:mainfrom
pascal-audio:instant-timers

Conversation

@lyager

@lyager lyager commented Oct 1, 2026

Copy link
Copy Markdown
Contributor

Problem

Every deadline the daemon keeps is derived from current_time_millis(), which reads SystemTime: 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::Instant for points in time and Duration for 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, probe start_time/next_send, ReRun/DelayedResponse::next_time, resolver timeouts, the timer heap) are Instant.
  • The 0 sentinels meaning "no time" (write_record, add_answer_at_time, the answers' per-record time, the IP-check timer) become Option<Instant>, since an Instant has no zero.
  • Deltas use the standard saturating operations (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 is timer.saturating_duration_since(now).max(1 ms).
  • TTLs on the wire are still remaining seconds measured from the record's creation, so packets are unchanged. The multicast rate limit keeps its u64 constant and is compared as a Duration at its single use site.
  • Seven const fns become fn, because Instant comparison 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 test passes (lib, integration, doc); cargo clippy --all-targets and cargo fmt --check are clean.
  • The device scenario above was reproduced and diagnosed against v0.21.4 (daemon registers at t≈2 s, kernel steps the clock back ~5 months at t≈3–7 s, no announce for the rest of the uptime). The fixed crate has not yet been run on that device; I will report back once it has.

Notes

  • Instant is CLOCK_MONOTONIC on 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. The SystemTime version 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.
  • A smaller alternative exists — keep current_time_millis() and anchor it to an Instant — 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.

@lyager
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
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 keepsimple1 left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for the PR! It's a real improvement, looks good!

@keepsimple1
keepsimple1 merged commit cba4ab4 into keepsimple1:main Oct 3, 2026
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants