From 91b5887f5dd3b3d9f715b358c29a7f64304e77d2 Mon Sep 17 00:00:00 2001 From: Luke Curley Date: Fri, 9 Oct 2026 19:31:13 -0700 Subject: [PATCH 1/4] feat(proto): add a handshake idle timeout Until the handshake completes, the idle timer uses TransportConfig::handshake_idle_timeout (default 10s, as msquic's HandshakeIdleTimeoutMs) instead of max_idle_timeout, then switches to the negotiated idle timeout once established. Before an RTT sample the PTO is 999ms, so a 2s idle timeout was armed at 3 * PTO = 2997ms while the second Initial probe is due at 2997ms plus timer slop. A client with a short idle timeout, chosen to detect a dead peer quickly, got only two Initial flights before giving up. A handshake to an unreachable peer now fails after the handshake idle timeout rather than the idle timeout; set both for the old behavior. Co-Authored-By: Claude Opus 5.5 --- noq-proto/src/config/transport.rs | 15 +++++++++++ noq-proto/src/connection/mod.rs | 19 ++++++++++---- noq-proto/src/tests/mod.rs | 42 +++++++++++++++++++++++++++++++ noq/src/tests.rs | 1 + 4 files changed, 72 insertions(+), 5 deletions(-) diff --git a/noq-proto/src/config/transport.rs b/noq-proto/src/config/transport.rs index 734e0ca315..44b9955eac 100644 --- a/noq-proto/src/config/transport.rs +++ b/noq-proto/src/config/transport.rs @@ -31,6 +31,7 @@ pub struct TransportConfig { pub(crate) max_concurrent_bidi_streams: VarInt, pub(crate) max_concurrent_uni_streams: VarInt, pub(crate) max_idle_timeout: Option, + pub(crate) handshake_idle_timeout: Duration, pub(crate) stream_receive_window: VarInt, pub(crate) receive_window: VarInt, pub(crate) send_window: u64, @@ -118,6 +119,16 @@ impl TransportConfig { self } + /// Maximum duration of inactivity to accept before abandoning a handshake. + /// + /// Used in place of [`max_idle_timeout`](Self::max_idle_timeout) until the handshake + /// completes, so a short idle timeout meant to detect a dead peer quickly does not also cut + /// a handshake off after a single probe. Defaults to 10 seconds. + pub fn handshake_idle_timeout(&mut self, value: Duration) -> &mut Self { + self.handshake_idle_timeout = value; + self + } + /// Maximum number of bytes the peer may transmit without acknowledgement on any one stream /// before becoming blocked. /// @@ -558,6 +569,8 @@ impl Default for TransportConfig { max_concurrent_uni_streams: 100u32.into(), // 30 second default recommended by RFC 9308 ยง 3.2 max_idle_timeout: Some(VarInt(30_000)), + // msquic's HandshakeIdleTimeoutMs default + handshake_idle_timeout: Duration::from_secs(10), stream_receive_window: STREAM_RWND.into(), receive_window: VarInt::MAX, send_window: (8 * STREAM_RWND).into(), @@ -610,6 +623,7 @@ impl fmt::Debug for TransportConfig { max_concurrent_bidi_streams, max_concurrent_uni_streams, max_idle_timeout, + handshake_idle_timeout, stream_receive_window, receive_window, send_window, @@ -647,6 +661,7 @@ impl fmt::Debug for TransportConfig { s.field("max_concurrent_bidi_streams", max_concurrent_bidi_streams) .field("max_concurrent_uni_streams", max_concurrent_uni_streams) .field("max_idle_timeout", max_idle_timeout) + .field("handshake_idle_timeout", handshake_idle_timeout) .field("stream_receive_window", stream_receive_window) .field("receive_window", receive_window) .field("send_window", send_window) diff --git a/noq-proto/src/connection/mod.rs b/noq-proto/src/connection/mod.rs index 208540e7a3..5f2c297edc 100644 --- a/noq-proto/src/connection/mod.rs +++ b/noq-proto/src/connection/mod.rs @@ -3913,11 +3913,15 @@ impl Connection { /// enabled there is an additional per-path idle timeout. fn reset_idle_timeout(&mut self, now: Instant, space: SpaceKind, path_id: PathId) { // First reset the global idle timeout. - if let Some(timeout) = self.idle_timeout { - if self.state.is_closed() { - self.timers - .stop(Timer::Conn(ConnTimer::Idle), self.qlog.with_time(now)); - } else { + let timeout = if self.state.is_closed() { + None + } else if self.state.is_handshake() { + Some(self.config.handshake_idle_timeout) + } else { + self.idle_timeout + }; + match timeout { + Some(timeout) => { let dt = cmp::max(timeout, 3 * self.max_pto_for_space(space)); self.timers.set( Timer::Conn(ConnTimer::Idle), @@ -3925,6 +3929,9 @@ impl Connection { self.qlog.with_time(now), ); } + None => self + .timers + .stop(Timer::Conn(ConnTimer::Idle), self.qlog.with_time(now)), } // Now handle the per-path state. @@ -4800,6 +4807,8 @@ impl Connection { self.events.push_back(Event::Connected); self.state.move_to_established(); + // Swap the handshake idle timeout for the negotiated one. + self.reset_idle_timeout(now, SpaceKind::Data, path_id); trace!("established"); // Multipath can only be enabled after the state has reached Established. diff --git a/noq-proto/src/tests/mod.rs b/noq-proto/src/tests/mod.rs index bc0be067e6..8e90ea225f 100644 --- a/noq-proto/src/tests/mod.rs +++ b/noq-proto/src/tests/mod.rs @@ -2871,6 +2871,48 @@ fn lost_finished_after_initial_backoff() { assert!(elapsed < Duration::from_secs(1), "{elapsed:?}"); } +/// A short idle timeout does not cut the handshake to two Initial flights. +/// +/// Before an RTT sample the PTO is 999ms, so a 2s idle timeout is armed at 2997ms, while +/// the second probe is due 999ms + 1998ms after the first send, plus however late the first +/// probe went out. The handshake idle timeout governs until the handshake completes. +#[test] +fn handshake_outlasts_short_idle_timeout() { + let _guard = subscribe(); + let mut pair = Pair::default(); + let mut transport = TransportConfig::default(); + transport.max_idle_timeout(Some(Duration::from_secs(2).try_into().unwrap())); + let mut client = client_config(); + client.transport_config(Arc::new(transport)); + + let client_ch = pair.begin_connect(client); + let start = pair.time; + // The first flight is lost, and so is the first probe, which goes out 2ms late. + pair.drive_client(); + pair.server.inbound.clear(); + pair.time = pair.client.next_wakeup().unwrap() + Duration::from_millis(2); + loop { + pair.drive_client(); + pair.server.inbound.clear(); + pair.time = pair.client.next_wakeup().unwrap(); + // Past any pacing: the second probe, due 2999ms after the first send, beyond the + // old 2997ms idle deadline. + if pair.time - start > Duration::from_secs(2) { + break; + } + } + + pair.drive(); + assert_matches!( + pair.client_conn_mut(client_ch).poll(), + Some(Event::HandshakeDataReady) + ); + assert_matches!( + pair.client_conn_mut(client_ch).poll(), + Some(Event::Connected) + ); +} + /// Ensures that the server can respond with 3 initial packets during the handshake /// before the anti-amplification limit kicks in when MTUs are similar. #[test] diff --git a/noq/src/tests.rs b/noq/src/tests.rs index f4bc8e9afa..882523229d 100755 --- a/noq/src/tests.rs +++ b/noq/src/tests.rs @@ -66,6 +66,7 @@ fn handshake_timeout() { let mut transport_config = TransportConfig::default(); transport_config .max_idle_timeout(Some(IDLE_TIMEOUT.try_into().unwrap())) + .handshake_idle_timeout(IDLE_TIMEOUT) .initial_rtt(Duration::from_millis(10)); client_config.transport_config(Arc::new(transport_config)); From 9f30274a609d3a15eabc918dc84d8b12a601b248 Mon Sep 17 00:00:00 2001 From: Luke Curley Date: Fri, 9 Oct 2026 20:21:07 -0700 Subject: [PATCH 2/4] fix(proto): apply the handshake idle timeout to Initials waiting for accept A server that delays Incoming::accept still dropped stale Initials by max_idle_timeout, so the handshake limit did not cover that wait, and with no idle timeout an Initial could wait forever. Co-Authored-By: Claude Opus 5.5 --- noq-proto/src/endpoint.rs | 13 ++++--------- noq-proto/src/tests/mod.rs | 20 ++++++++++++++++++++ 2 files changed, 24 insertions(+), 9 deletions(-) diff --git a/noq-proto/src/endpoint.rs b/noq-proto/src/endpoint.rs index d4a29c33b2..c857fa3896 100644 --- a/noq-proto/src/endpoint.rs +++ b/noq-proto/src/endpoint.rs @@ -15,8 +15,8 @@ use thiserror::Error; use tracing::{debug, error, trace, warn}; use crate::{ - Duration, FourTuple, INITIAL_MTU, Instant, MAX_CID_SIZE, MIN_INITIAL_SIZE, PathId, - RESET_TOKEN_SIZE, ResetToken, Side, Transmit, TransportConfig, TransportError, + FourTuple, INITIAL_MTU, Instant, MAX_CID_SIZE, MIN_INITIAL_SIZE, PathId, RESET_TOKEN_SIZE, + ResetToken, Side, Transmit, TransportConfig, TransportError, cid_generator::ConnectionIdGenerator, coding::{BufMutExt, Decodable, Encodable, UnexpectedEnd}, config::{ClientConfig, EndpointConfig, ServerConfig}, @@ -567,13 +567,8 @@ impl Endpoint { let server_config = server_config.unwrap_or_else(|| self.server_config.as_ref().unwrap().clone()); - if server_config - .transport - .max_idle_timeout - .is_some_and(|timeout| { - incoming.received_at + Duration::from_millis(timeout.into()) <= now - }) - { + // The handshake starts when its Initial arrives, not when it is accepted. + if incoming.received_at + server_config.transport.handshake_idle_timeout <= now { debug!("abandoning accept of stale initial"); self.index.remove_initial(dst_cid); return Err(Box::new(AcceptError { diff --git a/noq-proto/src/tests/mod.rs b/noq-proto/src/tests/mod.rs index 8e90ea225f..2a7f60d63c 100644 --- a/noq-proto/src/tests/mod.rs +++ b/noq-proto/src/tests/mod.rs @@ -2913,6 +2913,26 @@ fn handshake_outlasts_short_idle_timeout() { ); } +/// An Initial left waiting for `accept` past the handshake idle timeout is stale, even +/// though the idle timeout has not expired. +#[test] +fn stale_initial_uses_handshake_idle_timeout() { + let _guard = subscribe(); + let mut pair = Pair::default(); + pair.server.handle_incoming = Box::new(|_| IncomingConnectionBehavior::Wait); + pair.begin_connect(client_config()); + pair.drive_client(); + pair.drive_server(); + let incoming = pair.server.waiting_incoming.pop().unwrap(); + + // Past the 10s handshake idle timeout, inside the 30s idle timeout. + pair.time += Duration::from_secs(11); + assert_matches!( + pair.server.try_accept(incoming, pair.time), + Err(ConnectionError::TimedOut) + ); +} + /// Ensures that the server can respond with 3 initial packets during the handshake /// before the anti-amplification limit kicks in when MTUs are similar. #[test] From e6b21af2311201af76888c5936f15b979b4940ea Mon Sep 17 00:00:00 2001 From: Luke Curley Date: Fri, 9 Oct 2026 20:28:54 -0700 Subject: [PATCH 3/4] fix(proto): cap handshake probes by the handshake idle timeout pto_time_and_space picked the probe interval cap from the idle timeout, while the handshake is now bounded by handshake_idle_timeout, so a short handshake timeout with a long or disabled idle timeout got the slow 2s cap. Co-Authored-By: Claude Opus 5.5 --- noq-proto/src/connection/mod.rs | 9 ++++++++- noq-proto/src/tests/mod.rs | 35 +++++++++++++++++++++++++++++++++ 2 files changed, 43 insertions(+), 1 deletion(-) diff --git a/noq-proto/src/connection/mod.rs b/noq-proto/src/connection/mod.rs index 5f2c297edc..d59d5fa1c1 100644 --- a/noq-proto/src/connection/mod.rs +++ b/noq-proto/src/connection/mod.rs @@ -3633,11 +3633,18 @@ impl Connection { let path = self.path(path_id)?; let pto_count = path.pto_count; + // The idle timeout the probes have to beat, as `reset_idle_timeout` arms it. + let idle = if self.state.is_handshake() { + Some(self.config.handshake_idle_timeout) + } else { + path.idle_timeout.or(self.idle_timeout) + }; + // Cap the maximum interval between two tail-loss probes. let max_interval = if path.rtt.get() > SLOW_RTT_THRESHOLD { // For slow links we want to increase the interval beyond 2s. (path.rtt.get() * 3) / 2 - } else if let Some(idle) = path.idle_timeout.or(self.idle_timeout) + } else if let Some(idle) = idle && idle <= MIN_IDLE_FOR_FAST_PTO { // If the idle timeout is relatively low, cap at 1s so we get plenty of retries diff --git a/noq-proto/src/tests/mod.rs b/noq-proto/src/tests/mod.rs index 2a7f60d63c..6751836482 100644 --- a/noq-proto/src/tests/mod.rs +++ b/noq-proto/src/tests/mod.rs @@ -2913,6 +2913,41 @@ fn handshake_outlasts_short_idle_timeout() { ); } +/// The handshake idle timeout picks the cap on the probe interval during the handshake, so a +/// short one gets the fast cap even with the idle timeout disabled. +#[test] +fn handshake_idle_timeout_caps_probe_interval() { + let _guard = subscribe(); + let mut pair = Pair::default(); + let mut transport = TransportConfig::default(); + transport + .max_idle_timeout(None) + .handshake_idle_timeout(Duration::from_secs(7)); + let mut client = client_config(); + client.transport_config(Arc::new(transport)); + + let client_ch = pair.begin_connect(client); + let start = pair.time; + // The first flight and the probes at 1s and 3s are lost. + while pair.time - start < Duration::from_secs(5) { + pair.drive_client(); + pair.server.inbound.clear(); + pair.time = pair.client.next_wakeup().unwrap(); + } + + // The fast cap sends the next probe at about 6.05s, inside the 7s handshake idle timeout. + // The normal 2s cap would send it at about 7.05s, after the client gave up. + pair.drive(); + assert_matches!( + pair.client_conn_mut(client_ch).poll(), + Some(Event::HandshakeDataReady) + ); + assert_matches!( + pair.client_conn_mut(client_ch).poll(), + Some(Event::Connected) + ); +} + /// An Initial left waiting for `accept` past the handshake idle timeout is stale, even /// though the idle timeout has not expired. #[test] From 231c30d55772d3852647a69436d4d2122e0d8b41 Mon Sep 17 00:00:00 2001 From: Luke Curley Date: Fri, 9 Oct 2026 20:33:13 -0700 Subject: [PATCH 4/4] fix(proto): never fire a handshake deadline too far out to represent handshake_idle_timeout is an unbounded Duration, so Duration::MAX overflowed the Instant addition in Endpoint::accept and reset_idle_timeout. Such a deadline now never fires. Co-Authored-By: Claude Opus 5.5 --- noq-proto/src/connection/mod.rs | 11 +++++++---- noq-proto/src/endpoint.rs | 6 +++++- noq-proto/src/tests/mod.rs | 16 ++++++++++++++++ 3 files changed, 28 insertions(+), 5 deletions(-) diff --git a/noq-proto/src/connection/mod.rs b/noq-proto/src/connection/mod.rs index d59d5fa1c1..181b54988d 100644 --- a/noq-proto/src/connection/mod.rs +++ b/noq-proto/src/connection/mod.rs @@ -3927,12 +3927,15 @@ impl Connection { } else { self.idle_timeout }; - match timeout { - Some(timeout) => { - let dt = cmp::max(timeout, 3 * self.max_pto_for_space(space)); + // A deadline too far out to represent never fires. + let deadline = timeout.and_then(|timeout| { + now.checked_add(cmp::max(timeout, 3 * self.max_pto_for_space(space))) + }); + match deadline { + Some(deadline) => { self.timers.set( Timer::Conn(ConnTimer::Idle), - now + dt, + deadline, self.qlog.with_time(now), ); } diff --git a/noq-proto/src/endpoint.rs b/noq-proto/src/endpoint.rs index c857fa3896..1b9f3d46bd 100644 --- a/noq-proto/src/endpoint.rs +++ b/noq-proto/src/endpoint.rs @@ -568,7 +568,11 @@ impl Endpoint { server_config.unwrap_or_else(|| self.server_config.as_ref().unwrap().clone()); // The handshake starts when its Initial arrives, not when it is accepted. - if incoming.received_at + server_config.transport.handshake_idle_timeout <= now { + if incoming + .received_at + .checked_add(server_config.transport.handshake_idle_timeout) + .is_some_and(|deadline| deadline <= now) + { debug!("abandoning accept of stale initial"); self.index.remove_initial(dst_cid); return Err(Box::new(AcceptError { diff --git a/noq-proto/src/tests/mod.rs b/noq-proto/src/tests/mod.rs index 6751836482..4fdf517ce9 100644 --- a/noq-proto/src/tests/mod.rs +++ b/noq-proto/src/tests/mod.rs @@ -2948,6 +2948,22 @@ fn handshake_idle_timeout_caps_probe_interval() { ); } +/// A handshake idle timeout too long to add to an `Instant` never fires, instead of panicking. +#[test] +fn unbounded_handshake_idle_timeout() { + let _guard = subscribe(); + let mut transport = TransportConfig::default(); + transport.handshake_idle_timeout(Duration::MAX); + let transport = Arc::new(transport); + let mut server = server_config(); + server.transport = transport.clone(); + let mut client = client_config(); + client.transport_config(transport); + + let mut pair = Pair::new(Default::default(), server); + pair.connect_with(client); +} + /// An Initial left waiting for `accept` past the handshake idle timeout is stale, even /// though the idle timeout has not expired. #[test]