Fix StreamPriorityKey::cmp implementation so it doesn't violate Rust's Ord trait contract while keeping the intended round-robin behavior.

Author: PSUdaemon

quiche/src/h3/mod.rs

@@ -3825,33 +3825,30 @@ mod tests {
             more_frames: true,
         };
         assert_eq!(ev, ev_headers);
+        assert_eq!(s.poll_server(), Ok((0, Event::Data)));
+        assert_eq!(s.recv_body_server(0, &mut recv_buf), Ok(body.len()));
+        assert_eq!(s.poll_client(), Err(Error::Done));
+        assert_eq!(s.recv_body_server(0, &mut recv_buf), Ok(body.len()));
+        assert_eq!(s.poll_server(), Ok((0, Event::Finished)));
 
         let (_, ev) = s.poll_server().unwrap();
         let ev_headers = Event::Headers {
             list: reqs[1].clone(),
             more_frames: true,
         };
         assert_eq!(ev, ev_headers);
+        assert_eq!(s.poll_server(), Ok((4, Event::Data)));
+        assert_eq!(s.recv_body_server(4, &mut recv_buf), Ok(body.len()));
+        assert_eq!(s.poll_client(), Err(Error::Done));
+        assert_eq!(s.recv_body_server(4, &mut recv_buf), Ok(body.len()));
+        assert_eq!(s.poll_server(), Ok((4, Event::Finished)));
 
         let (_, ev) = s.poll_server().unwrap();
         let ev_headers = Event::Headers {
             list: reqs[2].clone(),
             more_frames: true,
         };
         assert_eq!(ev, ev_headers);
-
-        assert_eq!(s.poll_server(), Ok((0, Event::Data)));
-        assert_eq!(s.recv_body_server(0, &mut recv_buf), Ok(body.len()));
-        assert_eq!(s.poll_client(), Err(Error::Done));
-        assert_eq!(s.recv_body_server(0, &mut recv_buf), Ok(body.len()));
-        assert_eq!(s.poll_server(), Ok((0, Event::Finished)));
-
-        assert_eq!(s.poll_server(), Ok((4, Event::Data)));
-        assert_eq!(s.recv_body_server(4, &mut recv_buf), Ok(body.len()));
-        assert_eq!(s.poll_client(), Err(Error::Done));
-        assert_eq!(s.recv_body_server(4, &mut recv_buf), Ok(body.len()));
-        assert_eq!(s.poll_server(), Ok((4, Event::Finished)));
-
         assert_eq!(s.poll_server(), Ok((8, Event::Data)));
         assert_eq!(s.recv_body_server(8, &mut recv_buf), Ok(body.len()));
         assert_eq!(s.poll_client(), Err(Error::Done));

quiche/src/lib.rs

@@ -429,8 +429,6 @@ use crate::recovery::OnLossDetectionTimeoutOutcome;
 use crate::recovery::RecoveryOps;
 use crate::recovery::ReleaseDecision;
 
-use crate::stream::StreamPriorityKey;
-
 /// The current QUIC wire version.
 pub const PROTOCOL_VERSION: u32 = PROTOCOL_VERSION_V1;
 
@@ -4947,14 +4945,14 @@ impl<F: BufFactory> Connection<F> {
                 }
 
                 let priority_key = Arc::clone(&stream.priority_key);
+
                 // If the stream is no longer flushable, remove it from the queue
                 if !stream.is_flushable() {
                     self.streams.remove_flushable(&priority_key);
                 } else if stream.incremental {
-                    // Shuffle the incremental stream to the back of the
-                    // queue.
-                    self.streams.remove_flushable(&priority_key);
-                    self.streams.insert_flushable(&priority_key);
+                    // Cycle the incremental stream to the back of the queue
+                    // for round-robin scheduling.
+                    self.streams.cycle_flushable(stream_id);
                 }
 
                 #[cfg(feature = "fuzzing")]
@@ -5685,15 +5683,21 @@ impl<F: BufFactory> Connection<F> {
         stream.urgency = urgency;
         stream.incremental = incremental;
 
-        let new_priority_key = Arc::new(StreamPriorityKey {
-            urgency: stream.urgency,
-            incremental: stream.incremental,
+        let old_priority_key = Arc::clone(&stream.priority_key);
+
+        let sequence = self.streams.next_sequence();
+
+        let stream = self.streams.get_mut(stream_id).unwrap();
+
+        let new_priority_key = Arc::new(stream::StreamPriorityKey {
+            urgency,
+            incremental,
             id: stream_id,
+            sequence,
             ..Default::default()
         });
 
-        let old_priority_key =
-            std::mem::replace(&mut stream.priority_key, new_priority_key.clone());
+        stream.priority_key = Arc::clone(&new_priority_key);
 
         self.streams
             .update_priority(&old_priority_key, &new_priority_key);