From 77386badc34a80792714c890235885685814e832 Mon Sep 17 00:00:00 2001 From: n0-grookie <322172879+n0-grookie@users.noreply.github.com> Date: Mon, 31 Aug 2026 10:10:44 -0400 Subject: [PATCH 1/2] test(proto): add Pair::drive_until_timer Advances virtual time and drives both endpoints until a given timer is no longer armed on one side. `Pair::drive` steps while either endpoint has work, and `Connection::is_idle` counts a pending idle timer as no work -- so `drive` stops short of an idle timeout rather than stepping onto it, and `advance_time` on its own only jumps to the earliest timer pending anywhere. Reaching a timeout therefore needs its own loop. `timer_pending` is the accessor this needs; `timer` becomes `pub(crate)` so `src/tests` can name `Timer` and `PathTimer`. --- noq-proto/src/connection/mod.rs | 11 ++++++++++- noq-proto/src/tests/util.rs | 35 +++++++++++++++++++++++++++++++++ 2 files changed, 45 insertions(+), 1 deletion(-) diff --git a/noq-proto/src/connection/mod.rs b/noq-proto/src/connection/mod.rs index 7971c74d11..5a89682297 100644 --- a/noq-proto/src/connection/mod.rs +++ b/noq-proto/src/connection/mod.rs @@ -103,7 +103,7 @@ pub use streams::{ ShouldTransmit, StreamEvent, Streams, WriteError, }; -mod timer; +pub(crate) mod timer; use timer::{Timer, TimerTable}; mod transmit_buf; @@ -455,6 +455,15 @@ impl Connection { self.timers.peek() } + /// Returns the instant at which `timer` is armed to fire, or `None` if it is not. + /// + /// `None` covers the timer never having been armed, having fired, and having been + /// cancelled. + #[cfg(test)] + pub(crate) fn timer_pending(&self, timer: Timer) -> Option { + self.timers.get(timer) + } + /// Returns application-facing events /// /// Connections should be polled for events after: diff --git a/noq-proto/src/tests/util.rs b/noq-proto/src/tests/util.rs index 56070e6242..90af6ea8e0 100644 --- a/noq-proto/src/tests/util.rs +++ b/noq-proto/src/tests/util.rs @@ -20,6 +20,7 @@ use rustls::{ use tracing::{debug, info, info_span, trace}; use crate::crypto::rustls::{QuicClientConfig, QuicServerConfig, configured_provider}; +use crate::connection::timer::Timer; use crate::{ ClientConfig, ClosePathError, ClosedPath, Connection, ConnectionError, ConnectionEvent, ConnectionHandle, ConnectionStats, DatagramEvent, Datagrams, Dir, Duration, EcnCodepoint, @@ -766,6 +767,40 @@ impl ConnPair { self.conn_mut(side).poll_timeout() } + /// Advances virtual time and drives both endpoints until `timer` fires on `side`. + /// + /// [`Self::drive`] refuses to step past a point where the only timers still pending + /// are idle timers (see `Connection::is_idle`), so it cannot be used to reach an idle + /// timeout: the timeout is exactly what `is_idle` discounts. This loop keeps stepping + /// to each next wakeup until `timer` is no longer armed. + /// + /// Note that timers fire in `drive_client`/`drive_server`, not in `advance_time`, so + /// every step has to drive both endpoints. A timer that is *cancelled* rather than + /// fired also ends the loop; callers should still assert on the event they expect, so + /// that a cancellation fails loudly instead of passing silently. + /// + /// Each step advances the clock *before* driving, so packets that must be sent at the + /// current time have to be driven out by a preceding [`Self::drive`]; otherwise they + /// are queued until after the first step. + /// + /// Panics if the timer is still armed after 1024 steps. + #[track_caller] + pub(super) fn drive_until_timer(&mut self, side: Side, timer: Timer) { + let mut steps = 0; + while self.conn(side).timer_pending(timer).is_some() { + // If `timer` is armed, this endpoint has a wakeup scheduled, so the advance + // cannot run out of timers. + assert!( + self.advance_time(), + "{timer:?} is armed but no endpoint has a wakeup scheduled" + ); + self.drive_client(); + self.drive_server(); + steps += 1; + assert!(steps < 1024, "{timer:?} still armed after {steps} steps"); + } + } + pub(super) fn poll(&mut self, side: Side) -> Option { self.conn_mut(side).poll() } From 6c0d7ea09373156e35dea6f976fc7dd7a5eebd75 Mon Sep 17 00:00:00 2001 From: n0-grookie <322172879+n0-grookie@users.noreply.github.com> Date: Mon, 31 Aug 2026 10:12:47 -0400 Subject: [PATCH 2/2] test(proto): fix flaky open_path_validation_fails_server_side Failed in ~0.4% of runs (82 of 20000 endpoint seeds in a seeded sweep), which is how it surfaced in daily CI on FreeBSD: `poll()` returned `None` instead of the expected `PathEvent::Abandoned { reason: TimedOut }`. https://github.com/n0-computer/noq/actions/runs/33142246096 The test advanced with a single `advance_time()`, which jumps to the earliest timer pending on *either* endpoint, and assumed that jump lands on the 8s path-idle deadline of the unreachable path. When the client happens to do a routine key update shortly before that deadline -- PN phase exhaustion, which depends on the randomly chosen initial packet number -- the server arms `KeyDiscard` at 3x PTO, and that timer fires about 100ms earlier. The jump lands there instead, `drive` then stops because `is_idle` discounts the idle timers that remain, and virtual time never reaches the deadline. The path-idle timer stays armed the whole time, so nothing is lost: the implementation is correct, the single jump was the bug. Seed-dependent rather than platform-dependent -- the same seeds fail on Linux, and the sweep goes to 0 of 50000 with `drive_until_timer`. --- noq-proto/src/tests/multipath.rs | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/noq-proto/src/tests/multipath.rs b/noq-proto/src/tests/multipath.rs index 2b00fd542c..de1cdc689e 100644 --- a/noq-proto/src/tests/multipath.rs +++ b/noq-proto/src/tests/multipath.rs @@ -9,6 +9,7 @@ use assert_matches::assert_matches; use testresult::TestResult; use tracing::info; +use crate::connection::timer::{PathTimer, Timer}; use crate::{ ClientConfig, ConnectionId, ConnectionIdGenerator, Endpoint, EndpointConfig, FourTuple, LOCAL_CID_COUNT, NetworkChangeHint, PathId, PathStatus, RandomConnectionIdGenerator, @@ -451,10 +452,15 @@ fn open_path_validation_fails_server_side() -> TestResult { info!("manual keep-alive of PathId::ZERO"); pair.ping_path(Client, PathId::ZERO)?; + // Sent here, before the clock moves: `drive_until_timer` advances time before it + // drives, so a queued keep-alive would otherwise go out at the deadline itself. pair.drive(); info!("advancing time to past client path {path_id} idle"); - pair.advance_time(); + pair.drive_until_timer(Client, Timer::PerPath(path_id, PathTimer::PathIdle)); + // Deliver the abandonment to the server: the timer fires while driving the client, and + // the loop drives the server before that packet arrives. Without this step the + // `poll(Server)` check below would pass simply because the server never heard about it. pair.drive(); // The client gave up first and timed out.