diff --git a/noq-proto/src/connection/mod.rs b/noq-proto/src/connection/mod.rs index 898bd58a7..b7b10bfd4 100644 --- a/noq-proto/src/connection/mod.rs +++ b/noq-proto/src/connection/mod.rs @@ -3388,8 +3388,9 @@ impl Connection { ); } // Before removing the path, we fetch the final path stats via `Self::path_stats`. - // This updates some values for the last time. - let path_stats = self.path_stats.discard(&path_id); + // This ensures snapshot values (like rtt) are properly updated. + let path_stats = self.path_stats(path_id).unwrap_or_default(); + self.path_stats.discard(&path_id); self.partial_stats += path_stats; self.paths.remove(&path_id); self.spaces[SpaceId::Data].number_spaces.remove(&path_id); diff --git a/noq-proto/src/connection/stats.rs b/noq-proto/src/connection/stats.rs index dfaf02748..d065e9972 100644 --- a/noq-proto/src/connection/stats.rs +++ b/noq-proto/src/connection/stats.rs @@ -361,7 +361,7 @@ impl PathStatsMap { /// Removes the stats for a given path. /// /// Only do this once you are discarding the path. - pub(super) fn discard(&mut self, path_id: &PathId) -> PathStats { - self.0.remove(path_id).unwrap_or_default() + pub(super) fn discard(&mut self, path_id: &PathId) { + self.0.remove(path_id); } } diff --git a/noq-proto/src/tests/multipath.rs b/noq-proto/src/tests/multipath.rs index 4a45c37a6..04ed0cf0e 100644 --- a/noq-proto/src/tests/multipath.rs +++ b/noq-proto/src/tests/multipath.rs @@ -2228,3 +2228,48 @@ fn regression_delayed_path_cids_blocked() -> TestResult { } Ok(()) } + +#[test] +fn regression_discarded_path_stats_are_up_to_date() -> TestResult { + let _guard = subscribe(); + let mut pair = ConnPair::builder().enable_multipath().connect(); + + let server_addr = pair.routes.public_server_addr(); + let path_id = pair.open_path( + Client, + FourTuple::from_remote(server_addr), + PathStatus::Available, + )?; + pair.drive(); + + // Drain establishment events. + while pair.poll(Client).is_some() {} + while pair.poll(Server).is_some() {} + + // Close the path and drive until both sides discard it. + pair.close_path(Client, path_id, 0u8.into())?; + pair.drive(); + + assert_matches!( + pair.poll(Client), + Some(Event::Path(PathEvent::Abandoned { id, .. })) if id == path_id + ); + assert_matches!( + pair.poll(Server), + Some(Event::Path(PathEvent::Abandoned { id, .. })) if id == path_id + ); + + pair.drive(); + + let discarded_stats = assert_matches!( + pair.poll(Client), + Some(Event::Path(PathEvent::Discarded { id, path_stats })) if id == path_id + => *path_stats + ); + + // After a full handshake + MTU probing on the second path, these must be non-zero. + assert_ne!(discarded_stats.cwnd, 0); + assert_ne!(discarded_stats.current_mtu, 0); + + Ok(()) +}