A test to trigger 'earliest > now' assertion

neqo-http3/src/connection_client.rs

@@ -2620,7 +2620,7 @@ mod tests {
         force_idle(&mut client, &mut server);
 
         let idle_timeout = ConnectionParameters::default().get_idle_timeout();
-        assert_eq!(client.process_output(now()).callback(), idle_timeout / 2);
+        assert_eq!(client.process_output(now()).callback(), idle_timeout);
     }
 
     // Helper function: read response when a server sends HTTP_RESPONSE_2.
@@ -5115,7 +5115,7 @@ mod tests {
         assert!(!fin);
 
         force_idle(&mut client, &mut server);
-        assert_eq!(client.process_output(now()).callback(), idle_timeout / 2);
+        assert_eq!(client.process_output(now()).callback(), idle_timeout);
     }
 
     #[test]

neqo-transport/src/connection/idle.rs

@@ -46,23 +46,18 @@ impl IdleTimeout {
         self.timeout = min(self.timeout, peer_timeout);
     }
 
-    pub fn expiry(&self, now: Instant, pto: Duration, keep_alive: bool) -> Instant {
-        let start = match self.state {
+    fn start(&self, now: Instant) -> Instant {
+        match self.state {
             IdleTimeoutState::Init => now,
             IdleTimeoutState::PacketReceived(t) | IdleTimeoutState::AckElicitingPacketSent(t) => t,
-        };
-        let delay = if keep_alive && !self.keep_alive_outstanding {
-            // For a keep-alive timer, wait for half the timeout interval, but be sure
-            // not to wait too little or we will send many unnecessary probes.
-            max(self.timeout / 2, pto)
-        } else {
-            max(self.timeout, pto * 3)
-        };
-        qtrace!(
-            "IdleTimeout::expiry@{now:?} pto={pto:?}, ka={keep_alive} => {t:?}",
-            t = start + delay
-        );
-        start + delay
+        }
+    }
+
+    pub fn expiry(&self, now: Instant, pto: Duration) -> Instant {
+        let delay = max(self.timeout, pto * 3);
+        let t = self.start(now) + delay;
+        qtrace!("IdleTimeout::expiry@{now:?} pto={pto:?} => {t:?}");
+        t
     }
 
     pub fn on_packet_sent(&mut self, now: Instant) {
@@ -92,7 +87,13 @@ impl IdleTimeout {
     }
 
     pub fn expired(&self, now: Instant, pto: Duration) -> bool {
-        now >= self.expiry(now, pto, false)
+        now >= self.expiry(now, pto)
+    }
+
+    fn keep_alive_timeout(&self, now: Instant, pto: Duration) -> Instant {
+        // For a keep-alive timer, wait for half the timeout interval, but be sure
+        // not to wait too little or we will send many unnecessary probes.
+        self.start(now) + max(self.timeout / 2, pto)
     }
 
     pub fn send_keep_alive(
@@ -101,7 +102,7 @@ impl IdleTimeout {
         pto: Duration,
         tokens: &mut Vec<RecoveryToken>,
     ) -> bool {
-        if !self.keep_alive_outstanding && now >= self.expiry(now, pto, true) {
+        if !self.keep_alive_outstanding && now >= self.keep_alive_timeout(now, pto) {
             self.keep_alive_outstanding = true;
             tokens.push(RecoveryToken::KeepAlive);
             true

neqo-transport/src/connection/mod.rs

@@ -1029,8 +1029,7 @@ impl Connection {
             let rtt = path.rtt();
             let pto = rtt.pto(PacketNumberSpace::ApplicationData);
 
-            let keep_alive = self.streams.need_keep_alive();
-            let idle_time = self.idle_timeout.expiry(now, pto, keep_alive);
+            let idle_time = self.idle_timeout.expiry(now, pto);
             qtrace!([self], "Idle/keepalive timer {:?}", idle_time);
             delays.push(idle_time);
 

neqo-transport/src/connection/tests/idle.rs

@@ -415,9 +415,8 @@ fn keep_alive_initiator() {
     let stream = create_stream_idle(&mut server, &mut client);
     let mut now = now();
 
-    // Marking the stream for keep-alive changes the idle timeout.
     server.stream_keep_alive(stream, true).unwrap();
-    assert_idle(&mut server, now, default_timeout() / 2);
+    assert_idle(&mut server, now, default_timeout());
 
     // Wait that long and the server should send a PING frame.
     now += default_timeout() / 2;
@@ -431,8 +430,8 @@ fn keep_alive_initiator() {
     let out = server.process(out.as_ref(), now).dgram();
     assert!(client.process(out.as_ref(), now).dgram().is_none());
 
-    // Check that there will be next keep-alive ping after default_timeout() / 2.
-    assert_idle(&mut server, now, default_timeout() / 2);
+    // Check that there will be next keep-alive ping after default_timeout().
+    assert_idle(&mut server, now, default_timeout());
     now += default_timeout() / 2;
     let pings_before2 = server.stats().frame_tx.ping;
     let ping = server.process_output(now).dgram();
@@ -449,9 +448,8 @@ fn keep_alive_lost() {
     let stream = create_stream_idle(&mut server, &mut client);
     let mut now = now();
 
-    // Marking the stream for keep-alive changes the idle timeout.
     server.stream_keep_alive(stream, true).unwrap();
-    assert_idle(&mut server, now, default_timeout() / 2);
+    assert_idle(&mut server, now, default_timeout());
 
     // Wait that long and the server should send a PING frame.
     now += default_timeout() / 2;
@@ -480,7 +478,7 @@ fn keep_alive_lost() {
     // return some small timeout for the recovry although it does not have
     // any outstanding data. Therefore we call it after AT_LEAST_PTO.
     now += AT_LEAST_PTO;
-    assert_idle(&mut server, now, default_timeout() / 2 - AT_LEAST_PTO);
+    assert_idle(&mut server, now, default_timeout() - AT_LEAST_PTO);
 }
 
 /// The other peer can also keep it alive.
@@ -492,9 +490,8 @@ fn keep_alive_responder() {
     let stream = create_stream_idle(&mut server, &mut client);
     let mut now = now();
 
-    // Marking the stream for keep-alive changes the idle timeout.
     client.stream_keep_alive(stream, true).unwrap();
-    assert_idle(&mut client, now, default_timeout() / 2);
+    assert_idle(&mut client, now, default_timeout());
 
     // Wait that long and the client should send a PING frame.
     now += default_timeout() / 2;
@@ -513,7 +510,7 @@ fn keep_alive_unmark() {
     let stream = create_stream_idle(&mut client, &mut server);
 
     client.stream_keep_alive(stream, true).unwrap();
-    assert_idle(&mut client, now(), default_timeout() / 2);
+    assert_idle(&mut client, now(), default_timeout());
 
     client.stream_keep_alive(stream, false).unwrap();
     assert_idle(&mut client, now(), default_timeout());
@@ -543,11 +540,11 @@ fn keep_alive_close() {
     let stream = create_stream_idle(&mut client, &mut server);
 
     client.stream_keep_alive(stream, true).unwrap();
-    assert_idle(&mut client, now(), default_timeout() / 2);
+    assert_idle(&mut client, now(), default_timeout());
 
     client.stream_close_send(stream).unwrap();
     transfer_force_idle(&mut client, &mut server);
-    assert_idle(&mut client, now(), default_timeout() / 2);
+    assert_idle(&mut client, now(), default_timeout());
 
     server.stream_close_send(stream).unwrap();
     transfer_force_idle(&mut server, &mut client);
@@ -564,11 +561,11 @@ fn keep_alive_reset() {
     let stream = create_stream_idle(&mut client, &mut server);
 
     client.stream_keep_alive(stream, true).unwrap();
-    assert_idle(&mut client, now(), default_timeout() / 2);
+    assert_idle(&mut client, now(), default_timeout());
 
     client.stream_close_send(stream).unwrap();
     transfer_force_idle(&mut client, &mut server);
-    assert_idle(&mut client, now(), default_timeout() / 2);
+    assert_idle(&mut client, now(), default_timeout());
 
     server.stream_reset_send(stream, 0).unwrap();
     transfer_force_idle(&mut server, &mut client);
@@ -590,7 +587,7 @@ fn keep_alive_stop_sending() {
     let stream = create_stream_idle(&mut client, &mut server);
 
     client.stream_keep_alive(stream, true).unwrap();
-    assert_idle(&mut client, now(), default_timeout() / 2);
+    assert_idle(&mut client, now(), default_timeout());
 
     client.stream_close_send(stream).unwrap();
     client.stream_stop_sending(stream, 0).unwrap();
@@ -614,14 +611,14 @@ fn keep_alive_multiple_stop() {
     let stream = create_stream_idle(&mut client, &mut server);
 
     client.stream_keep_alive(stream, true).unwrap();
-    assert_idle(&mut client, now(), default_timeout() / 2);
+    assert_idle(&mut client, now(), default_timeout());
 
     let other = client.stream_create(StreamType::BiDi).unwrap();
     client.stream_keep_alive(other, true).unwrap();
-    assert_idle(&mut client, now(), default_timeout() / 2);
+    assert_idle(&mut client, now(), default_timeout());
 
     client.stream_keep_alive(stream, false).unwrap();
-    assert_idle(&mut client, now(), default_timeout() / 2);
+    assert_idle(&mut client, now(), default_timeout());
 
     client.stream_keep_alive(other, false).unwrap();
     assert_idle(&mut client, now(), default_timeout());
@@ -692,9 +689,8 @@ fn keep_alive_with_ack_eliciting_packet_lost() {
 
     // Create a stream.
     let stream = client.stream_create(StreamType::BiDi).unwrap();
-    // Marking the stream for keep-alive changes the idle timeout.
     client.stream_keep_alive(stream, true).unwrap();
-    assert_idle(&mut client, now, IDLE_TIMEOUT / 2);
+    assert_idle(&mut client, now, IDLE_TIMEOUT);
 
     // Send data on the stream that will be lost.
     _ = client.stream_send(stream, DEFAULT_STREAM_DATA).unwrap();
@@ -709,13 +705,11 @@ fn keep_alive_with_ack_eliciting_packet_lost() {
     let retransmit = client.process_output(now).dgram();
     assert!(retransmit.is_some());
 
-    // The next callback should be for an idle PING.
-    assert_eq!(
-        client.process_output(now).callback(),
-        IDLE_TIMEOUT / 2 - pto
-    );
+    // The timeout is the twice the PTO, because we've already sent one probe.
+    assert_eq!(client.process_output(now).callback(), pto * 2);
 
-    // Wait that long and the client should send a PING frame.
+    // Wait for half the idle timeout (less the PTO we've already waited)
+    // so that we get a keep-alive.
     now += IDLE_TIMEOUT / 2 - pto;
     let pings_before = client.stats().frame_tx.ping;
     let ping = client.process_output(now).dgram();
@@ -742,3 +736,25 @@ fn keep_alive_with_ack_eliciting_packet_lost() {
     assert!(matches!(out, Output::None));
     assert!(matches!(client.state(), State::Closed(_)));
 }
+
+#[test]
+fn keep_alive_with_unresponsive_server() {
+    let mut client = default_client();
+    let mut server = default_server();
+    connect(&mut client, &mut server);
+
+    let mut now = now();
+    let client_stream = client.stream_create(StreamType::BiDi).unwrap();
+    client.stream_keep_alive(client_stream, true).unwrap();
+
+    for _ in 0..100 {
+        if client.stream_send(client_stream, &[0x0; 500]).is_err() {
+            break;
+        }
+        if let Output::Callback(t) = client.process_output(now) {
+            now += t;
+        }
+    }
+    // Connection should be closed due to idle timeout.
+    assert!(matches!(client.state(), State::Closed(_)));
+}