Skip to content

Commit 592e56a

Browse files
committed
Avoid underflow panic in packet loss Instant calculations
1 parent 350d6bc commit 592e56a

File tree

1 file changed

+16
-4
lines changed
  • quinn-proto/src/connection

1 file changed

+16
-4
lines changed

quinn-proto/src/connection/mod.rs

Lines changed: 16 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1712,8 +1712,6 @@ impl Connection {
17121712
let rtt = self.path.rtt.conservative();
17131713
let loss_delay = cmp::max(rtt.mul_f32(self.config.time_threshold), TIMER_GRANULARITY);
17141714

1715-
// Packets sent before this time are deemed lost.
1716-
let lost_send_time = now.checked_sub(loss_delay).unwrap();
17171715
let largest_acked_packet = self.spaces[pn_space].largest_acked_packet.unwrap();
17181716
let packet_threshold = self.config.packet_threshold as u64;
17191717
let mut size_of_lost_packets = 0u64;
@@ -1737,8 +1735,11 @@ impl Connection {
17371735
persistent_congestion_start = None;
17381736
}
17391737

1740-
if info.time_sent <= lost_send_time || largest_acked_packet >= packet + packet_threshold
1741-
{
1738+
// Packets sent before now - loss_delay are deemed lost.
1739+
// However, we avoid this subtraction as it can panic and there's no
1740+
// saturating equivalent of this substraction operation with a Duration.
1741+
let packet_too_old = now.saturating_duration_since(info.time_sent) >= loss_delay;
1742+
if packet_too_old || largest_acked_packet >= packet + packet_threshold {
17421743
if Some(packet) == in_flight_mtu_probe {
17431744
// Lost MTU probes are not included in `lost_packets`, because they should not
17441745
// trigger a congestion control response
@@ -1791,6 +1792,17 @@ impl Connection {
17911792
lost_packets, size_of_lost_packets
17921793
);
17931794

1795+
// Packets sent before this time are deemed lost.
1796+
// We avoid computing this value above, since it's possible for this to panic
1797+
// if the `loss_delay` value internally stores a bigger `Duration` than the
1798+
// `Duration` that's stored inside the `Instant`, because some platforms may
1799+
// implement the `Instant` with a counter relative to system or even process
1800+
// startup (Wasm is one such case).
1801+
// If we're at this point, then it must be possible to have instants that are
1802+
// longer ago than `loss_delay` (see the `packet_too_old` computation
1803+
// above).
1804+
let lost_send_time = now.checked_sub(loss_delay).unwrap();
1805+
17941806
for &packet in &lost_packets {
17951807
let info = self.spaces[pn_space].take(packet).unwrap(); // safe: lost_packets is populated just above
17961808
self.config.qlog_sink.emit_packet_lost(

0 commit comments

Comments
 (0)