From 96ed282a3509950ff683a154c582259854dfe6f4 Mon Sep 17 00:00:00 2001 From: "Youri T. K. K. Mattar" Date: Sun, 9 Aug 2026 14:30:55 -0300 Subject: [PATCH] feat(timer): calendar schedules on the wall clock, and datagram sockets MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Closes M14, and closes the roadmap. Two clocks, because two kinds of deadline mean different things. A backoff, a stop timeout and a start timeout are all "this long from now" and must not move when the clock is corrected: CLOCK_MONOTONIC, relative. A calendar schedule is "at 03:30" and must stay at 03:30 however the clock gets there: CLOCK_REALTIME, and absolute. M8 collapsed the second into the first — it computed "in fourteen hours" once and armed a monotonic timer with it, so an NTP correction moved a nightly job by however much the clock moved. The fix is not to recompute on a step, it is to stop converting: an absolute deadline needs no recomputation because the kernel compares it against the same clock that changed. TFD_TIMER_CANCEL_ON_SET is set too and is NOT what makes that true; it only makes the step visible, worth a line in the log. Saying so matters, because the roadmap entry asking for this named the flag as though the flag were the fix. Schedule now returns which clock it means — Next::After(Duration) or Next::At(Unix) — rather than a delay that has already lost the distinction. That is what makes the routing impossible to get wrong. A datagram socket is a different shape: never listened on, never accepted, so oxinit starts the service on a readable descriptor rather than a pending connection, and the datagram that woke it is still queued for the service to read. listen-probe grew --recv and --send; the stream path could not have exercised any of it. seqpacket stays uncovered on purpose — it is accepted like a stream, so a fixture for it would test the fixture. Corrects M13's deferral note, which was wrong: the backoff reset is and was covered, by two host tests in oxinit-service, which is exactly where policy logic with no OS dependency belongs. Also drops two stale claims in UNIT_FORMAT.md: "nothing here is implemented yet", true until M1, and "one of three kinds", true until M8. 41 checks per architecture, 43 against the distribution image, 148 host tests. --- ARCHITECTURE.md | 24 ++++-- README.md | 10 ++- ROADMAP.md | 89 ++++++++++++++++---- crates/listen-probe/src/main.rs | 111 +++++++++++++++++++------ crates/oxinit-timer/src/lib.rs | 123 ++++++++++++++++++++++------ crates/oxinit/src/event.rs | 6 +- crates/oxinit/src/main.rs | 2 + crates/oxinit/src/supervisor.rs | 66 +++++++++++---- crates/oxinit/src/timer.rs | 141 +++++++++++++++++++++++++++++++- crates/xtask/src/main.rs | 12 +++ docs/UNIT_FORMAT.md | 25 +++--- units/default.toml | 3 + units/knock-socket.toml | 13 +++ units/knock.toml | 8 ++ units/knocker.toml | 12 +++ units/nightly.toml | 11 +++ 16 files changed, 551 insertions(+), 105 deletions(-) create mode 100644 units/knock-socket.toml create mode 100644 units/knock.toml create mode 100644 units/knocker.toml create mode 100644 units/nightly.toml diff --git a/ARCHITECTURE.md b/ARCHITECTURE.md index 931a1a8..4f04e7f 100644 --- a/ARCHITECTURE.md +++ b/ARCHITECTURE.md @@ -318,7 +318,9 @@ there is no framing layer, no length prefix, and no partial-message state to get wrong. `SOCK_STREAM` would require all three; `SOCK_DGRAM` would drop the connection semantics needed to reply. -**Timerfd** — exactly one, paired with a `BinaryHeap`. Restart +**Timerfd** — one per clock, each paired with a `BinaryHeap`. The +monotonic one carries every relative deadline; the realtime one carries +absolute calendar moments. See [Timers](#timers). Restart backoff, start timeouts, and watchdog intervals all become entries in the heap. The timerfd is armed for the earliest deadline. When it fires, every expired entry is popped and dispatched, then it is rearmed. One fd per timer would mean @@ -827,11 +829,21 @@ not stop the schedule**: the timer is re-armed before the service is started, because a timer says *when*, and what to do about a failure is the service's `restart`. -**Known limitation.** A calendar deadline is computed once, as a delay on the -monotonic clock, so a clock step between arming and firing moves that firing. -Every firing after it is recomputed against the corrected clock. The proper -fix is a second timerfd on `CLOCK_REALTIME` with `TFD_TIMER_CANCEL_ON_SET`, -which is a second clock in the heap and has not been earned yet. +**Two clocks.** A backoff, a stop timeout and a start timeout are all "this +long from now" and must not move when the clock is corrected: `CLOCK_MONOTONIC`, +relative. A calendar schedule is "at 03:30" and must stay at 03:30 however the +clock gets there: `CLOCK_REALTIME`, and **absolute**. Two timerfds, a heap +each, both in the same epoll loop. + +Collapsing the second into the first — computing a delay once and arming a +monotonic timer with it — is what made an NTP correction move a nightly job. +The fix is to stop converting, not to recompute on a step: an absolute +deadline needs no recomputation, because the kernel compares it against the +same clock that changed. `TFD_TIMER_CANCEL_ON_SET` only makes the step +visible; it is not what makes the deadline right. + +`Schedule` returns which clock it means — `Next::After(Duration)` or +`Next::At(Unix)` — rather than a delay that has already lost the distinction. `oxinit-timer` holds the schedule types and the arithmetic, takes the current time as an argument, and makes no syscall — so "what does this do at 23:59:59" diff --git a/README.md b/README.md index 610ca50..fbafd2b 100644 --- a/README.md +++ b/README.md @@ -109,7 +109,9 @@ on-calendar = "03:30" `on-boot` and `interval` cover the monotonic cases — first firing, then every firing after it, measured from the previous one. The calendar vocabulary is deliberately closed: `hourly`, `daily`, `weekly`, `HH:MM`, `HH:MM:SS`, in UTC. -It is not a cron expression and will not become one. A firing that arrives +It is not a cron expression and will not become one. Calendar deadlines are +armed absolutely on a `CLOCK_REALTIME` timerfd, so "at 03:30" survives the +clock being corrected under it. A firing that arrives while the service is still running is skipped rather than queued, and a failed run does not stop the schedule. @@ -203,7 +205,7 @@ getting it wrong strands the machine. ## Current state -Milestones 0 through 13 are done. oxinit boots under QEMU and runs as a +Milestones 0 through 14 are done, and the roadmap is closed. oxinit boots under QEMU and runs as a container's PID 1. | | | @@ -222,12 +224,14 @@ container's PID 1. | **M11** | `after`, `requires` and `conflicts` enforced; `start-sec` bounds activation. | | **M12** | CI: every suite above, on every push. | | **M13** | Coverage for seven behaviours the milestones claimed and nothing re-ran. | +| **M14** | Calendar schedules on the wall clock; datagram sockets; the list closed. | One `epoll` loop multiplexes the signalfd, the timerfd, the notify socket, the control socket, every socket unit's listening descriptor and every service cgroup's `cgroup.events`. One thread. No async runtime. -Nothing is scheduled after M13. [ROADMAP.md](ROADMAP.md) has the breakdown, +Nothing is scheduled after M14, and [ROADMAP.md](ROADMAP.md) says which two +remaining ideas are deliberately not being taken and why. [ROADMAP.md](ROADMAP.md) has the breakdown, including what each milestone was verified against and what was deferred out of it. diff --git a/ROADMAP.md b/ROADMAP.md index db07209..f28b213 100644 --- a/ROADMAP.md +++ b/ROADMAP.md @@ -792,21 +792,76 @@ watching it. Verified on both architectures, in the container, and against the real Alpine userspace: 38 checks per boot, 40 against the distribution image. -Deferred out of M13: the backoff *reset* after a sustained `Active` period, -which M2 also claims. Distinguishing "reset" from "never incremented" needs an -assertion about a counter's history, and substring matching over a log cannot -make one. It needs a different kind of test than this suite is. - -## Beyond M13 - -Not planned, not designed, listed only so the questions have an answer: - -- Coverage for `type = "datagram"` and `type = "seqpacket"` sockets, which - needs a fixture that speaks them; `listen-probe` only does streams. -- A `CLOCK_REALTIME` timerfd with `TFD_TIMER_CANCEL_ON_SET`, so a calendar - schedule survives the clock being stepped under it. Until then a wall-clock - firing is computed as a monotonic delay and an NTP correction moves it by - however much the clock moved. -- Local time, which means a timezone database. -- Replacing JSON with `postcard` on the control socket. +Deferred out of M13: nothing. This entry originally said the backoff *reset* +was uncovered and needed "a different kind of test than this suite is". That +was wrong, and M14 found it: `oxinit-service` has had +`backoff_resets_after_the_service_stays_up` and +`a_brief_active_period_does_not_reset_backoff` all along, in exactly the right +place. The reset is policy logic in a crate with no OS dependency, and a host +test is what it should be tested by — the boot suite was never the thing +missing. + +## M14 — The loose ends + +**Done.** + +Everything left on the list, and an honest account of the two things that are +staying off it. + +- [x] Calendar schedules on a `CLOCK_REALTIME` timerfd, armed absolutely. +- [x] `type = "datagram"` sockets, activated and asserted. +- [x] The M13 note about the backoff reset, which was wrong. + +**Two clocks, because two kinds of deadline mean different things.** A backoff, +a stop timeout and a start timeout are all "this long from now" and must not +move when the clock is corrected — monotonic, relative. A calendar schedule is +"at 03:30" and must stay at 03:30 however the clock gets there — realtime, +*absolute*. + +M8 collapsed the second into the first: it computed "in fourteen hours" once +and armed a monotonic timer with it, so an NTP correction moved a nightly job +by however much the clock moved. The fix is not to recompute on a clock step — +it is to stop converting. An absolute realtime deadline needs no +recomputation, because the kernel compares it against the same clock that +changed. + +`TFD_TIMER_CANCEL_ON_SET` is set as well and is *not* what makes that true. It +only makes the step visible: the read fails with `ECANCELED` once, worth a line +in the log and nothing else. Saying so matters, because the roadmap entry that +asked for this named the flag as though the flag were the fix. + +`Schedule` now returns which clock it means — `Next::After(Duration)` or +`Next::At(Unix)` — rather than a delay that has already lost the distinction. +That is the whole of the type change, and it is what makes the routing +impossible to get wrong. + +**A datagram socket is a different shape.** It is never `listen`ed on and never +accepted, so oxinit starts the service on a *readable* descriptor rather than a +pending connection, and the datagram that woke it is still queued for the +service to read. `listen-probe` grew `--recv` and `--send` for it; the stream +path could not have exercised any of that. + +`type = "seqpacket"` is still uncovered, and deliberately: it is `accept`ed +like a stream, so it exercises the same oxinit path the `stream` units already +do. A fixture for it would test the fixture. + +Verified: 41 checks per architecture, 43 against the distribution image, 148 +host tests. + +## Not doing, and why + +These were on the list. They are coming off it with a reason rather than +sitting there forever. + +**`postcard` instead of JSON on the control socket.** ARCHITECTURE sets the +precondition itself — "the likely replacement *once the message set stops +changing*" — and it has not. `UnitStatus` gained a field in M8 and another in +M14. JSON's stated benefit, that a message is readable with `socat` while the +protocol is still moving, is worth more than bytes on a socket carrying a few +messages a minute. The crate boundary is placed so this stays a one-file change +whenever it is worth making. + +**Local time.** It means carrying a timezone database, which contradicts a +position the project has already taken in writing, twice. Calendar schedules +are UTC and say so. Reopening that is a decision, not a loose end. - A test suite that boots a real distribution userspace rather than a shell. diff --git a/crates/listen-probe/src/main.rs b/crates/listen-probe/src/main.rs index 3622d77..c89bd0a 100644 --- a/crates/listen-probe/src/main.rs +++ b/crates/listen-probe/src/main.rs @@ -7,10 +7,18 @@ //! Usage: //! //! ```text -//! listen-probe --serve NAME # the activated service -//! listen-probe --connect PATH # the client that triggers it +//! listen-probe --serve NAME # the activated service, over a stream +//! listen-probe --recv NAME # the activated service, over a datagram +//! listen-probe --connect PATH # the client that triggers a stream +//! listen-probe --send PATH # the client that triggers a datagram //! ``` //! +//! A datagram socket is never `listen`ed on and never accepts, so oxinit +//! watches it and starts the service on a *readable* descriptor rather than on +//! a pending connection — and the service reads the datagram that is already +//! queued rather than accepting anything. Different enough to be worth its own +//! pair of modes. +//! //! `--serve NAME` looks its descriptor up by name in `LISTEN_FDNAMES` rather //! than assuming fd 3, which is what the variable is for: a service given //! descriptors by more than one socket unit has to be able to tell them apart. @@ -18,7 +26,7 @@ use std::io::{Read as _, Write as _}; use std::net::Shutdown; use std::os::fd::FromRawFd as _; -use std::os::unix::net::{UnixListener, UnixStream}; +use std::os::unix::net::{UnixDatagram, UnixListener, UnixStream}; use std::process::ExitCode; /// Where the protocol says the first inherited descriptor is. @@ -27,10 +35,16 @@ const FIRST_LISTEN_FD: i32 = 3; fn main() -> ExitCode { let args: Vec = std::env::args().collect(); - let result = match (flag(&args, "--serve"), flag(&args, "--connect")) { - (Some(name), _) => serve(name), - (None, Some(path)) => connect(path), - (None, None) => Err("usage: listen-probe --serve NAME | --connect PATH".to_owned()), + let result = if let Some(name) = flag(&args, "--serve") { + serve(name) + } else if let Some(name) = flag(&args, "--recv") { + recv(name) + } else if let Some(path) = flag(&args, "--connect") { + connect(path) + } else if let Some(path) = flag(&args, "--send") { + send(path) + } else { + Err("usage: listen-probe --serve|--recv NAME | --connect|--send PATH".to_owned()) }; match result { @@ -44,6 +58,37 @@ fn main() -> ExitCode { /// Serve exactly one connection on the descriptor named `name`. fn serve(name: &str) -> Result<(), String> { + let index = inherited(name)?; + + // SAFETY: the descriptor was inherited from oxinit at this number, is not + // owned by anything else in this process, and the protocol says it is a + // listening socket. This is a test fixture, not PID 1. + let listener = unsafe { UnixListener::from_raw_fd(FIRST_LISTEN_FD + index) }; + + let (mut stream, _) = listener.accept().map_err(|e| format!("accept: {e}"))?; + + let mut request = [0u8; 64]; + let read = stream + .read(&mut request) + .map_err(|e| format!("read: {e}"))?; + let request = String::from_utf8_lossy(request.get(..read).unwrap_or_default()).into_owned(); + + println!("listen-probe: serving a connection, request {request:?}"); + + stream + .write_all(format!("echo: {request}").as_bytes()) + .map_err(|e| format!("write: {e}"))?; + + Ok(()) +} + +/// Find the descriptor named `name`, checking the protocol's one hard rule on +/// the way. +/// +/// Returns the offset from [`FIRST_LISTEN_FD`], not the descriptor: looking it +/// up by name is the whole point of `LISTEN_FDNAMES`, and a service handed +/// descriptors by more than one socket unit cannot assume fd 3. +fn inherited(name: &str) -> Result { let count: usize = var("LISTEN_FDS")? .parse() .map_err(|_| "LISTEN_FDS is not a number".to_owned())?; @@ -66,31 +111,51 @@ fn serve(name: &str) -> Result<(), String> { .position(|candidate| candidate == name) .ok_or_else(|| format!("no descriptor named `{name}` in `{names}`"))?; + let index = i32::try_from(index).map_err(|_| "too many descriptors".to_owned())?; + println!( "listen-probe: {count} descriptor(s) as `{names}`, LISTEN_PID={announced} is mine, \ serving `{name}` on fd {}", - FIRST_LISTEN_FD + index as i32 + FIRST_LISTEN_FD + index ); - // SAFETY: the descriptor was inherited from oxinit at this number, is not - // owned by anything else in this process, and the protocol says it is a - // listening socket. This is a test fixture, not PID 1. - let listener = unsafe { UnixListener::from_raw_fd(FIRST_LISTEN_FD + index as i32) }; - - let (mut stream, _) = listener.accept().map_err(|e| format!("accept: {e}"))?; + Ok(index) +} - let mut request = [0u8; 64]; - let read = stream - .read(&mut request) - .map_err(|e| format!("read: {e}"))?; - let request = String::from_utf8_lossy(request.get(..read).unwrap_or_default()).into_owned(); +/// The datagram half of `serve`. +/// +/// The datagram that started this service is already in the socket's queue — +/// oxinit does not read it, exactly as it does not accept a connection. There +/// is nothing to accept and no peer to reply to unless the sender bound an +/// address, so this reports what it got and stops. +fn recv(name: &str) -> Result<(), String> { + let index = inherited(name)?; + + // SAFETY: same as `serve` — the descriptor was inherited at this number, + // nothing else in this process owns it, and this is a test fixture rather + // than PID 1. + let socket = unsafe { UnixDatagram::from_raw_fd(FIRST_LISTEN_FD + index) }; + + let mut buf = [0u8; 64]; + let read = socket.recv(&mut buf).map_err(|e| format!("recv: {e}"))?; + let payload = String::from_utf8_lossy(buf.get(..read).unwrap_or_default()).into_owned(); + + println!("listen-probe: datagram on `{name}`, got {payload:?}"); + Ok(()) +} - println!("listen-probe: serving a connection, request {request:?}"); +/// Send one datagram, which is what makes oxinit start the service. +fn send(path: &str) -> Result<(), String> { + let socket = UnixDatagram::unbound().map_err(|e| format!("socket: {e}"))?; - stream - .write_all(format!("echo: {request}").as_bytes()) - .map_err(|e| format!("write: {e}"))?; + socket + .send_to(b"knock", path) + .map_err(|e| format!("send to {path}: {e}"))?; + // Nothing to wait for. A datagram socket has no connection to hold open + // and this sender bound no address to be answered on, so the send is the + // whole of the client's side. + println!("listen-probe: sent a datagram to {path}"); Ok(()) } diff --git a/crates/oxinit-timer/src/lib.rs b/crates/oxinit-timer/src/lib.rs index 159566e..eac5576 100644 --- a/crates/oxinit-timer/src/lib.rs +++ b/crates/oxinit-timer/src/lib.rs @@ -199,6 +199,36 @@ pub struct Schedule { pub calendar: Option, } +/// When to fire, and *which clock says so*. +/// +/// The distinction is the whole of what a clock step does to a schedule. A +/// monotonic delay means "this long from now" and must not move when the clock +/// is corrected; an absolute wall-clock moment means "at 03:30" and must stay +/// at 03:30 however the clock gets there. Collapsing the second into the first +/// — computing a delay once and arming a monotonic timer with it — is what +/// made an NTP correction move a nightly job by however much the clock moved. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub enum Next { + /// A delay on the monotonic clock. + After(Duration), + /// A moment on the wall clock, in seconds since the epoch. + At(Unix), +} + +impl Next { + /// Roughly how far away this is, for a countdown. `now` is the wall clock. + /// + /// Approximate for the calendar half by exactly the amount the clock is + /// about to be corrected by, which is the honest answer to "how long until + /// 03:30" when nobody knows what time it really is. + pub fn in_seconds(self, now: Unix) -> u64 { + match self { + Next::After(delay) => delay.as_secs(), + Next::At(at) => at.saturating_sub(now), + } + } +} + impl Schedule { /// Nothing to fire on. A load-time error, not a timer that sits forever. pub fn is_empty(&self) -> bool { @@ -210,9 +240,8 @@ impl Schedule { /// `interval` with no `on-boot` means the first firing is one interval /// away, which is what "every ten minutes" means to the person who wrote /// it. - pub fn first(&self, now: Unix) -> Option { - let monotonic = self.on_boot.or(self.interval); - Self::sooner(monotonic, self.calendar_delay(now)) + pub fn first(&self, now: Unix) -> Option { + Self::sooner(self.on_boot.or(self.interval), self.calendar_at(now), now) } /// The delay to arm with after a firing. @@ -225,26 +254,37 @@ impl Schedule { /// `None` when the timer is done: an `on-boot` with no `interval` and no /// calendar is a one-shot delayed task, and re-arming it would turn a /// deliberate single firing into a loop. - pub fn next(&self, now: Unix) -> Option { - Self::sooner(self.interval, self.calendar_delay(now)) + pub fn next(&self, now: Unix) -> Option { + Self::sooner(self.interval, self.calendar_at(now), now) } - /// Seconds from `now` to the next calendar occurrence. + /// The next wall-clock moment this schedule names. /// /// `now + 1` because [`Calendar::next_at`] answers "at or after", and /// asking at the exact second a schedule names would otherwise return that /// same second and fire again immediately. - fn calendar_delay(&self, now: Unix) -> Option { - let calendar = self.calendar?; - let at = calendar.next_at(now.saturating_add(1)); - - Some(Duration::from_secs(at.saturating_sub(now))) + fn calendar_at(&self, now: Unix) -> Option { + Some(self.calendar?.next_at(now.saturating_add(1))) } - fn sooner(a: Option, b: Option) -> Option { - match (a, b) { - (Some(a), Some(b)) => Some(a.min(b)), - (some, None) | (None, some) => some, + /// Whichever comes first, keeping the clock it belongs to. + /// + /// Compared in wall-clock seconds because that is the only scale both are + /// expressible on. Sub-second precision is lost in the comparison and not + /// in the result: a monotonic delay that wins is returned as the delay it + /// was, not as a rounded moment. + fn sooner(delay: Option, at: Option, now: Unix) -> Option { + match (delay, at) { + (Some(delay), Some(at)) => { + if now.saturating_add(delay.as_secs()) <= at { + Some(Next::After(delay)) + } else { + Some(Next::At(at)) + } + } + (Some(delay), None) => Some(Next::After(delay)), + (None, Some(at)) => Some(Next::At(at)), + (None, None) => None, } } } @@ -264,6 +304,10 @@ mod tests { Some(Duration::from_secs(n)) } + fn after(n: u64) -> Option { + Some(Next::After(Duration::from_secs(n))) + } + #[test] fn parses_the_named_schedules() { assert_eq!(Calendar::parse("hourly"), Ok(Calendar::Hourly)); @@ -378,7 +422,7 @@ mod tests { ..Schedule::default() }; - assert_eq!(schedule.next(midnight), secs(DAY)); + assert_eq!(schedule.next(midnight), Some(Next::At(midnight + DAY))); } #[test] @@ -388,8 +432,8 @@ mod tests { ..Schedule::default() }; - assert_eq!(schedule.first(NOW), secs(600)); - assert_eq!(schedule.next(NOW), secs(600)); + assert_eq!(schedule.first(NOW), after(600)); + assert_eq!(schedule.next(NOW), after(600)); } #[test] @@ -399,7 +443,7 @@ mod tests { ..Schedule::default() }; - assert_eq!(schedule.first(NOW), secs(30)); + assert_eq!(schedule.first(NOW), after(30)); assert_eq!( schedule.next(NOW), None, @@ -415,8 +459,8 @@ mod tests { ..Schedule::default() }; - assert_eq!(schedule.first(NOW), secs(5)); - assert_eq!(schedule.next(NOW), secs(3600)); + assert_eq!(schedule.first(NOW), after(5)); + assert_eq!(schedule.next(NOW), after(3600)); } #[test] @@ -431,8 +475,41 @@ mod tests { assert_eq!( schedule.next(NOW), - secs(25 * MINUTE + 4), - "the hour comes before the day" + Some(Next::At(NOW + 25 * MINUTE + 4)), + "the hour comes before the day, and stays on the wall clock" + ); + } + + #[test] + fn a_calendar_firing_keeps_the_wall_clock() { + // The bug this exists to prevent: collapsing "at 03:30" into "in + // fourteen hours" and arming a monotonic timer with it, so that an NTP + // correction moves the job by however much the clock moved. + let schedule = Schedule { + calendar: Some(Calendar::At { + hour: 3, + minute: 30, + second: 0, + }), + ..Schedule::default() + }; + + assert!(matches!(schedule.next(NOW), Some(Next::At(_)))); + } + + #[test] + fn a_monotonic_firing_keeps_its_sub_second_precision() { + // Comparison happens in whole seconds; the result must not be rounded + // to them. + let schedule = Schedule { + interval: Some(Duration::from_millis(1500)), + calendar: Some(Calendar::Daily), + ..Schedule::default() + }; + + assert_eq!( + schedule.next(NOW), + Some(Next::After(Duration::from_millis(1500))) ); } diff --git a/crates/oxinit/src/event.rs b/crates/oxinit/src/event.rs index 60356d2..ef79249 100644 --- a/crates/oxinit/src/event.rs +++ b/crates/oxinit/src/event.rs @@ -14,8 +14,10 @@ use crate::error::{Error, Result}; pub enum Source { /// A signal is pending on the signalfd. Signals, - /// At least one deadline has expired. + /// At least one monotonic deadline has expired. Timer, + /// A wall-clock deadline arrived, or the clock was set under one. + WallTimer, /// A service sent an sd_notify datagram. Notify, /// A service cgroup's `populated` key changed. Carries the id the @@ -57,6 +59,7 @@ impl Source { Source::Control => (KIND_FIXED, 3), Source::LogSocket => (KIND_FIXED, 4), Source::LogShipper => (KIND_FIXED, 5), + Source::WallTimer => (KIND_FIXED, 6), Source::Cgroup(id) => (KIND_CGROUP, id), Source::Socket(id) => (KIND_SOCKET, id), Source::Client(id) => (KIND_CLIENT, id), @@ -74,6 +77,7 @@ impl Source { 3 => Some(Source::Control), 4 => Some(Source::LogSocket), 5 => Some(Source::LogShipper), + 6 => Some(Source::WallTimer), _ => None, }, KIND_CGROUP => Some(Source::Cgroup(id)), diff --git a/crates/oxinit/src/main.rs b/crates/oxinit/src/main.rs index 8143c78..d7fb13b 100644 --- a/crates/oxinit/src/main.rs +++ b/crates/oxinit/src/main.rs @@ -194,6 +194,7 @@ fn boot() -> ! { let registered = events .register(&signals, event::Source::Signals) .and_then(|()| events.register(supervisor.timers.as_fd(), event::Source::Timer)) + .and_then(|()| events.register(supervisor.timers.wall_fd(), event::Source::WallTimer)) .and_then(|()| events.register(supervisor.notify.as_fd(), event::Source::Notify)); if let Some(control) = control.as_ref() { @@ -306,6 +307,7 @@ fn run( let result = catch_unwind(AssertUnwindSafe(|| match source { event::Source::Signals => on_signals(signals, supervisor, shell, &mut pending), event::Source::Timer => supervisor.on_timer(), + event::Source::WallTimer => supervisor.on_wall_timer(), event::Source::Notify => supervisor.on_notify(), event::Source::Cgroup(id) => supervisor.on_cgroup(*id), event::Source::Socket(id) => supervisor.on_socket(*id), diff --git a/crates/oxinit/src/supervisor.rs b/crates/oxinit/src/supervisor.rs index 2bc3993..0fb0812 100644 --- a/crates/oxinit/src/supervisor.rs +++ b/crates/oxinit/src/supervisor.rs @@ -3,7 +3,7 @@ use std::collections::{BTreeMap, BTreeSet}; use std::os::fd::{BorrowedFd, OwnedFd}; use std::process::{Child, Command}; -use std::time::{Duration, Instant, SystemTime, UNIX_EPOCH}; +use std::time::{Duration, SystemTime, UNIX_EPOCH}; use rustix::fs::OFlags; use rustix::process::{Pid, Signal}; @@ -23,6 +23,7 @@ use crate::reap; use crate::shutdown::{Action, Shutdown}; use crate::sys::raw::{setup_child, ChildSetup, Image, FIRST_LISTEN_FD}; use crate::timer::{Alarm, Timers}; +use oxinit_timer::Next; pub struct Supervisor { instances: BTreeMap, @@ -47,10 +48,11 @@ pub struct Supervisor { /// the policy has nothing to say about it. Something else has to remember /// that a start was asked for, and this is it. pending_restart: BTreeSet, - /// When each armed timer unit next fires. Reported by `oxctl`, and kept - /// here rather than asked of the deadline heap, which is a `BinaryHeap` - /// and answers "the soonest" rather than "this one's". - next_elapse: BTreeMap, + /// When each armed timer unit next fires, on whichever clock its schedule + /// chose. Reported by `oxctl`, and kept here rather than asked of the + /// deadline heaps, which are `BinaryHeap`s and answer "the soonest" rather + /// than "this one's". + next_elapse: BTreeMap, /// Set once the machine is going down. Nothing starts after this, and /// every event is also a chance to notice that everything has stopped. shutdown: Option, @@ -522,8 +524,8 @@ impl Supervisor { /// `None` means there is no next one — an `on-boot` with no `interval` and /// no calendar is a delayed one-shot, and the unit stays `Active` with /// nothing pending rather than re-arming into a loop nobody asked for. - fn arm(&mut self, name: &str, service: &str, delay: Option) { - let Some(delay) = delay else { + fn arm(&mut self, name: &str, service: &str, next: Option) { + let Some(next) = next else { println!("oxinit: {name} has fired; nothing further scheduled"); self.next_elapse.remove(name); return; @@ -533,11 +535,26 @@ impl Supervisor { unit: name.to_owned(), }; - match self.timers.schedule(delay, alarm) { + // The clock is chosen by the schedule, not by this. A monotonic delay + // must not move when the clock is corrected; a wall-clock moment must + // stay the moment it is. + let armed = match next { + Next::After(delay) => self.timers.schedule(delay, alarm), + Next::At(at) => self.timers.schedule_at(at, alarm), + }; + + match armed { Ok(()) => { - self.next_elapse - .insert(name.to_owned(), Instant::now() + delay); - println!("oxinit: {name} will start {service} in {delay:?}"); + self.next_elapse.insert(name.to_owned(), next); + match next { + Next::After(delay) => { + println!("oxinit: {name} will start {service} in {delay:?}"); + } + Next::At(at) => { + let left = at.saturating_sub(unix_now()); + println!("oxinit: {name} will start {service} at {at} ({left}s away)"); + } + } } Err(e) => eprintln!("oxinit: {name}: {e}"), } @@ -1214,12 +1231,10 @@ impl Supervisor { status: instance.status.clone(), memory: stats.memory, tasks: stats.tasks, - next_elapse: self.next_elapse.get(instance.name()).map(|at| { - // Rounded up. A timer that has not fired yet must never report - // "in 0s", which reads as broken rather than as imminent. - let left = at.saturating_duration_since(Instant::now()); - left.as_secs() + u64::from(left.subsec_nanos() > 0) - }), + next_elapse: self + .next_elapse + .get(instance.name()) + .map(|next| next.in_seconds(unix_now()).max(1)), } } @@ -1275,6 +1290,23 @@ impl Supervisor { } /// Restart whatever the timers say is due. + /// The wall clock reached a calendar deadline, or was stepped. + pub fn on_wall_timer(&mut self) { + let fired = match self.timers.wall_expired(unix_now()) { + Ok(fired) => fired, + Err(e) => { + eprintln!("oxinit: {e}"); + return; + } + }; + + for alarm in fired { + if let Alarm::TimerElapsed { unit } = alarm { + self.timer_elapsed(&unit); + } + } + } + pub fn on_timer(&mut self) { let fired = match self.timers.expired() { Ok(fired) => fired, diff --git a/crates/oxinit/src/timer.rs b/crates/oxinit/src/timer.rs index b0e8f7c..a0ff247 100644 --- a/crates/oxinit/src/timer.rs +++ b/crates/oxinit/src/timer.rs @@ -1,9 +1,26 @@ //! Deadlines. //! -//! One timerfd and a heap, not one fd per timer. A restart storm across a -//! hundred services would otherwise mean a hundred descriptors and an epoll -//! registration per restart; here it means a hundred heap entries and one -//! `timerfd_settime`. +//! One timerfd per *clock*, and a heap each — not one fd per timer. A restart +//! storm across a hundred services would otherwise mean a hundred descriptors +//! and an epoll registration per restart; here it means a hundred heap entries +//! and one `timerfd_settime`. +//! +//! **Two clocks, because two kinds of deadline mean different things.** A +//! backoff, a stop timeout and a start timeout are all "this long from now", +//! and must not move when the clock is corrected — those are `CLOCK_MONOTONIC` +//! and relative. A calendar schedule is "at 03:30", and must stay at 03:30 +//! however the clock gets there — that is `CLOCK_REALTIME` and *absolute*. +//! +//! Collapsing the second into the first is what M8 shipped and M14 fixed: +//! computing "in fourteen hours" once and arming a monotonic timer with it +//! moves a nightly job by however much an NTP correction moves the clock. An +//! absolute realtime timer needs no recomputation, because the kernel is +//! comparing against the same clock that changed. +//! +//! `TFD_TIMER_CANCEL_ON_SET` is set as well, and is not what makes the above +//! true — it only makes the step *visible*: the read fails with `ECANCELED` +//! once, which is worth a line in the log and nothing else. The deadline is +//! still correct. use std::cmp::Reverse; use std::collections::BinaryHeap; @@ -50,9 +67,30 @@ impl PartialOrd for Entry { } } +/// A deadline on the wall clock, in seconds since the epoch. +#[derive(Debug, Clone, PartialEq, Eq)] +struct WallEntry { + at: u64, + alarm: Alarm, +} + +impl Ord for WallEntry { + fn cmp(&self, other: &Self) -> std::cmp::Ordering { + self.at.cmp(&other.at) + } +} + +impl PartialOrd for WallEntry { + fn partial_cmp(&self, other: &Self) -> Option { + Some(self.cmp(other)) + } +} + pub struct Timers { fd: OwnedFd, heap: BinaryHeap>, + wall: OwnedFd, + wall_heap: BinaryHeap>, } impl Timers { @@ -63,9 +101,17 @@ impl Timers { ) .map_err(Error::Timer)?; + let wall = rustix::time::timerfd_create( + TimerfdClockId::Realtime, + TimerfdFlags::CLOEXEC | TimerfdFlags::NONBLOCK, + ) + .map_err(Error::Timer)?; + Ok(Self { fd, heap: BinaryHeap::new(), + wall, + wall_heap: BinaryHeap::new(), }) } @@ -73,6 +119,10 @@ impl Timers { self.fd.as_fd() } + pub fn wall_fd(&self) -> impl AsFd + '_ { + self.wall.as_fd() + } + /// Schedule `alarm` for `delay` from now, and rearm if it is the earliest. pub fn schedule(&mut self, delay: Duration, alarm: Alarm) -> Result<()> { self.heap.push(Reverse(Entry { @@ -82,6 +132,71 @@ impl Timers { self.arm() } + /// Schedule `alarm` for a moment on the wall clock. + /// + /// Absolute, and stored absolute. A calendar schedule that was converted + /// to a delay here would be back to moving with the clock. + pub fn schedule_at(&mut self, at: u64, alarm: Alarm) -> Result<()> { + self.wall_heap.push(Reverse(WallEntry { at, alarm })); + self.arm_wall() + } + + /// Everything on the wall clock whose moment has passed. + /// + /// A clock step makes the read fail with `ECANCELED` — once — and nothing + /// else needs doing: the deadlines are absolute, so they are still the + /// moments they were. Rearming is what clears the condition. + pub fn wall_expired(&mut self, now: u64) -> Result> { + let mut buf = [0u8; 8]; + if let Err(rustix::io::Errno::CANCELED) = rustix::io::read(&self.wall, buf.as_mut_slice()) { + println!("oxinit: the wall clock was set; calendar deadlines are unchanged"); + self.arm_wall()?; + return Ok(Vec::new()); + } + + let mut fired = Vec::new(); + while let Some(Reverse(entry)) = self.wall_heap.peek() { + if entry.at > now { + break; + } + if let Some(Reverse(entry)) = self.wall_heap.pop() { + fired.push(entry.alarm); + } + } + + self.arm_wall()?; + Ok(fired) + } + + /// Point the realtime timerfd at the earliest wall-clock moment. + fn arm_wall(&self) -> Result<()> { + let Some(Reverse(entry)) = self.wall_heap.peek() else { + return disarm(&self.wall); + }; + + let spec = Itimerspec { + it_interval: Timespec { + tv_sec: 0, + tv_nsec: 0, + }, + it_value: Timespec { + tv_sec: entry.at as i64, + tv_nsec: 0, + }, + }; + + // ABSTIME is the point: the value is a moment, not a delay, so the + // kernel compares it against the clock as the clock is now rather than + // as it was when this was armed. CANCEL_ON_SET only asks to be told. + rustix::time::timerfd_settime( + &self.wall, + TimerfdTimerFlags::ABSTIME | TimerfdTimerFlags::CANCEL_ON_SET, + &spec, + ) + .map(|_| ()) + .map_err(Error::Timer) + } + /// Everything whose deadline has passed, and rearm for whatever is next. pub fn expired(&mut self) -> Result> { // Draining the fd matters: it is level-triggered, so an unread @@ -140,3 +255,21 @@ impl Timers { .map_err(Error::Timer) } } + +/// An all-zero `itimerspec` disarms a timerfd. +fn disarm(fd: &OwnedFd) -> Result<()> { + let spec = Itimerspec { + it_interval: Timespec { + tv_sec: 0, + tv_nsec: 0, + }, + it_value: Timespec { + tv_sec: 0, + tv_nsec: 0, + }, + }; + + rustix::time::timerfd_settime(fd, TimerfdTimerFlags::empty(), &spec) + .map(|_| ()) + .map_err(Error::Timer) +} diff --git a/crates/xtask/src/main.rs b/crates/xtask/src/main.rs index 9d6a90c..1515f6f 100644 --- a/crates/xtask/src/main.rs +++ b/crates/xtask/src/main.rs @@ -247,6 +247,14 @@ const EXPECTED: &[(&str, &str)] = &[ "M4: LISTEN_FDNAMES", ), ("got \"echo: hello\"", "M4: an activated service answered"), + ( + "listening for knock-socket on /run/knock.sock", + "M14: a datagram socket binds", + ), + ( + "datagram on `knock-socket`, got \"knock\"", + "M14: and a datagram activates the service, which reads what woke it", + ), ( "oxinit: logs: shipper connected", "M7: oxlogd took the log socket", @@ -267,6 +275,10 @@ const EXPECTED: &[(&str, &str)] = &[ "oxinit: tick-timer will start stamp in 2s", "M8: a timer arms with on-boot", ), + ( + "oxinit: nightly will start stamp at ", + "M14: a calendar schedule arms at a wall-clock moment, not in a delay", + ), ( "oxinit: tick-timer elapsed; starting stamp", "M8: and fires", diff --git a/docs/UNIT_FORMAT.md b/docs/UNIT_FORMAT.md index e717122..f4a2225 100644 --- a/docs/UNIT_FORMAT.md +++ b/docs/UNIT_FORMAT.md @@ -3,8 +3,9 @@ Units are TOML files. This document specifies every key: its type, whether it is required, its default, and what happens when it is absent. -Nothing here is implemented yet. This is the specification the parser in -`oxinit-unit` is written against. +This is the specification the parser in `oxinit-unit` is written against, not a +description of it. Changing the format means changing this document in the same +commit. ## File naming and location @@ -17,7 +18,7 @@ rejected at load time. ### Unit kinds -A unit is one of three kinds, determined by which section it carries. Exactly +A unit is one of four kinds, determined by which section it carries. Exactly one of these must be present: | Section | Kind | Status | @@ -31,8 +32,8 @@ Names are unique across kinds: there cannot be both a `sshd` service and a `sshd` target. That is what lets `requires`, `after`, and `oxctl` take a bare name with no suffix and no ambiguity. -This is why a socket unit names its service explicitly rather than sharing a -name with it. `sshd-socket` activating `sshd` is two units with two names; the +This is why a socket unit — and a timer unit — names its service explicitly +rather than sharing a name with it. `sshd-socket` activating `sshd` is two units with two names; the alternative would mean `requires = ["sshd"]` no longer identifies one unit. A unit's **fully-qualified name** is `.` — `sshd.service`, @@ -685,12 +686,14 @@ teaches an operator a syntax this document does not describe. **Times are UTC.** oxinit has no timezone database and is not going to carry one. -**A clock step moves a pending firing.** A calendar deadline is computed once, -as a delay on the monotonic clock, so an NTP correction between arming and -firing moves that firing by however much the clock moved. Every firing after -it is recomputed against the corrected clock. Fixing this properly needs a -`CLOCK_REALTIME` timerfd with `TFD_TIMER_CANCEL_ON_SET`; see -[ROADMAP.md](../ROADMAP.md). +**A clock step does not move a pending firing.** A calendar deadline is armed +absolutely, on a `CLOCK_REALTIME` timerfd, so "at 03:30" stays at 03:30 +however the clock gets there — including across an NTP correction that jumps +over it, which fires it immediately, and one that jumps back before it, which +does not. + +That is a property of arming absolutely rather than of noticing the step. +oxinit does notice, and logs it, and that is all the noticing is for. ## File descriptor passing diff --git a/units/default.toml b/units/default.toml index 42e0342..a3ad477 100644 --- a/units/default.toml +++ b/units/default.toml @@ -14,6 +14,7 @@ wants = [ "logged", "ticker", "tick-timer", + "nightly", "slow", "patient", "broken", @@ -36,6 +37,8 @@ wants = [ "echo-socket", "echo-tcp", "poke", + "knock-socket", + "knocker", "absent", ] after = ["console", "banner", "logged"] diff --git a/units/knock-socket.toml b/units/knock-socket.toml new file mode 100644 index 0000000..81e6587 --- /dev/null +++ b/units/knock-socket.toml @@ -0,0 +1,13 @@ +# A datagram socket. Never listen()ed on, never accepted: oxinit watches it and +# starts the service when it becomes *readable*, and the datagram that woke it +# is still queued for the service to read. +# +# `backlog` is refused alongside this type rather than ignored, which the +# parser has always done and no boot had ever exercised. +[unit] +description = "Knock socket, datagram" + +[socket] +listen = ["/run/knock.sock"] +service = "knock" +type = "datagram" diff --git a/units/knock.toml b/units/knock.toml new file mode 100644 index 0000000..929a6a6 --- /dev/null +++ b/units/knock.toml @@ -0,0 +1,8 @@ +# Started by a datagram, not by boot. +[unit] +description = "Datagram-activated service" + +[service] +type = "oneshot" +exec = "/bin/listen-probe --recv knock-socket" +restart = "no" diff --git a/units/knocker.toml b/units/knocker.toml new file mode 100644 index 0000000..ae39dda --- /dev/null +++ b/units/knocker.toml @@ -0,0 +1,12 @@ +# Sends the datagram that starts `knock`. +# +# Ordered after the socket unit, which guarantees the address is bound — the +# same fact `poke` relies on, and the reason binding happens before anything +# starts. +[unit] +description = "Sends a datagram to the knock socket" +after = ["knock-socket"] + +[service] +type = "oneshot" +exec = "/bin/listen-probe --send /run/knock.sock" diff --git a/units/nightly.toml b/units/nightly.toml new file mode 100644 index 0000000..208d657 --- /dev/null +++ b/units/nightly.toml @@ -0,0 +1,11 @@ +# A calendar schedule, so a boot exercises the wall-clock path. +# +# It will not fire inside a test image's uptime, and that is the point: what +# is asserted is that oxinit armed it *at* a moment rather than *in* a delay. +# A monotonic delay computed once is what an NTP correction used to move. +[unit] +description = "Hourly, on the hour" + +[timer] +service = "stamp" +on-calendar = "hourly"