fix(proto): On abandoned paths declare packets as lost after some time

noq-proto/src/connection/mod.rs

@@ -727,9 +727,15 @@ impl Connection {
                 // This timer should not be set, for completeness it's not kept as it's set when
                 // the PATH_ABANDON frame is sent.
                 PathTimer::PathDrained => false,
-                // Sent packets still need to be identified as lost to trigger timely
-                // retransmission.
-                PathTimer::LossDetection => true,
+                // Stopped so that `set_loss_detection_timer` below arms the
+                // declare-in-flight-lost deadline in its place: it only arms
+                // when the timer is unarmed. A leftover PTO or loss time
+                // would fire earlier, and on an abandoned path firing means
+                // declaring everything in flight lost, recreating the
+                // immediate retransmission the 2*PTO window avoids. The wait
+                // costs nothing: per-packet loss detection does not need this
+                // timer, it reruns on every ACK arriving during the window.
+                PathTimer::LossDetection => false,
                 // This path should not be used for sending after the PATH_ABANDON frame is sent.
                 // However, any outstanding data that should be sent before PATH_ABANDON, should
                 // still respect pacing.
@@ -742,10 +748,9 @@ impl Connection {
             }
         }
 
-        // Set the loss detection timer again, as now it should only be set
-        // for time-based loss detection, not tail-loss probes, but currently it
-        // could still be set to a tail-loss probe.
-        // This will reset it to the next time-based loss time, if applicable.
+        // Re-arm the loss detection timer: on the now-abandoned path it
+        // becomes the declare-in-flight-lost deadline (see
+        // `set_loss_detection_timer`).
         self.set_loss_detection_timer(now, path_id);
 
         // Emit event to the application.
@@ -3196,17 +3201,23 @@ impl Connection {
 
     /// Handle a [`PathTimer::LossDetection`] timeout.
     ///
-    /// This timer expires for two reasons:
+    /// This timer expires for one of three reasons:
     /// - An ACK-eliciting packet we sent should be considered lost.
     /// - The PTO may have expired and a tail-loss probe needs to be scheduled.
+    /// - The path was abandoned 2*PTO ago and its remaining in-flight packets
+    ///   should now be declared lost (see [`Connection::abandon_path`]).
     ///
-    /// The former needs us to schedule re-transmission of the lost data.
+    /// The first needs us to schedule re-transmission of the lost data.
     ///
-    /// The latter means we have not received an ACK for an ack-eliciting packet we sent
+    /// The second means we have not received an ACK for an ack-eliciting packet we sent
     /// within the PTO time-window. We need to schedule a tail-loss probe, an ack-eliciting
     /// packet, to try and elicit new acknowledgements. These new acknowledgements will
     /// indicate whether the previously sent packets were lost or not.
     fn on_loss_detection_timeout(&mut self, now: Instant, path_id: PathId) {
+        if self.abandoned_paths.contains(&path_id) {
+            self.declare_abandoned_in_flight_lost(now, path_id);
+            return;
+        }
         if let Some((_, pn_space)) = self.loss_time_and_space(path_id) {
             // Time threshold loss Detection
             self.detect_lost_packets(now, pn_space, path_id, false);
@@ -3243,6 +3254,52 @@ impl Connection {
         self.set_loss_detection_timer(now, path_id);
     }
 
+    /// Declares an abandoned path's remaining in-flight packets lost.
+    ///
+    /// Runs when the declare-in-flight-lost deadline fires, 2*PTO after the
+    /// abandon (see [`Connection::set_loss_detection_timer`]). Whatever the
+    /// peer acknowledged in the meantime (ACKs for this path keep arriving
+    /// over other paths) is already gone from `sent_packets`; the rest will
+    /// never be acknowledged and is requeued for retransmission on the
+    /// remaining paths.
+    fn declare_abandoned_in_flight_lost(&mut self, now: Instant, path_id: PathId) {
+        let in_flight_mtu_probe = self.path_data(path_id).mtud.in_flight_mtu_probe();
+        let mut size_of_lost_packets = 0u64;
+        let lost_pns: Vec<_> = self.spaces[SpaceId::Data]
+            .for_path(path_id)
+            .sent_packets
+            .iter()
+            .filter(|(pn, _info)| Some(*pn) != in_flight_mtu_probe)
+            .map(|(pn, info)| {
+                size_of_lost_packets += info.size as u64;
+                pn
+            })
+            .collect();
+
+        if !lost_pns.is_empty() {
+            trace!(
+                %path_id,
+                count = lost_pns.len(),
+                lost_bytes = size_of_lost_packets,
+                "declaring in-flight packets lost on abandoned path"
+            );
+            self.handle_lost_packets(
+                SpaceId::Data,
+                path_id,
+                now,
+                lost_pns,
+                in_flight_mtu_probe,
+                Duration::ZERO,
+                false,
+                size_of_lost_packets,
+            );
+        }
+        self.timers.stop(
+            Timer::PerPath(path_id, PathTimer::LossDetection),
+            self.qlog.with_time(now),
+        );
+    }
+
     /// Detect any lost packets
     ///
     /// There are two cases in which we detects lost packets:
@@ -3677,6 +3734,43 @@ impl Connection {
             return;
         }
 
+        if self.abandoned_paths.contains(&path_id) {
+            // On an abandoned path the timer is repurposed: it declares the
+            // path's remaining in-flight packets lost once 2*PTO have passed
+            // since the abandon (see `declare_abandoned_in_flight_lost`).
+            // Without it, that data would sit in `sent_packets` until
+            // something else frees it: normally the PathDrained timer, which
+            // only arms once the peer's own PATH_ABANDON for this path
+            // arrives, 3*PTO after that, and which needs some live path to
+            // carry the frame at all. Failing that, the path's idle timeout
+            // eventually frees the data, far later than a few PTOs.
+            //
+            // The delay exists because the in-flight packets may well have
+            // been delivered: their ACKs can still arrive, coalesced onto
+            // other paths (PathAck names the acked path explicitly,
+            // independent of the path carrying the frame). Declaring
+            // everything lost immediately would retransmit data the peer
+            // already has. 2*PTO is comfortably enough for any such ACK to
+            // make it back; only what is still unacknowledged then gets
+            // requeued onto the remaining paths.
+            //
+            // The deadline is armed by the `abandon_path` call ending up here
+            // and then left alone: a later call, such as one triggered by an
+            // ACK arriving during the window, must neither move nor stop it.
+            if self
+                .timers
+                .get(Timer::PerPath(path_id, PathTimer::LossDetection))
+                .is_none()
+            {
+                self.timers.set(
+                    Timer::PerPath(path_id, PathTimer::LossDetection),
+                    now + 2 * self.pto(SpaceKind::Data, path_id),
+                    self.qlog.with_time(now),
+                );
+            }
+            return;
+        }
+
         if let Some((loss_time, _)) = self.loss_time_and_space(path_id) {
             // Time threshold loss detection.
             self.timers.set(