Clean up streams in h3::Connection when done

maurer · maurer · commit eb617408c92a · 2025-09-23T22:38:05.000Z
Currently, while `quiche::Connection` will clean up streams from its
`StreamMap`, `quiche::h3::Connection` does not. As a result, long lived
connections will slowly leak memory through the stream map in the
connection.

This patch forwards the `collect()` event from the `quiche::Connection`
to the `quiche::h3::Connection` through the `h3::Connection::poll()`
call, allowing the `h3::Connection` to clean up its unused streams.
diff --git a/quiche/src/h3/mod.rs b/quiche/src/h3/mod.rs
@@ -2070,6 +2070,10 @@ impl Connection {
             return Ok((finished, Event::Finished));
         }
 
+        for stream_id in conn.streams.collected_streams() {
+            self.streams.remove(&stream_id);
+        }
+
         Err(Error::Done)
     }
 
@@ -3211,6 +3215,13 @@ impl Connection {
                 .decoder_stream_bytes,
         }
     }
+
+    #[cfg(test)]
+    /// Indicates how many streams are currently being tracked for use
+    /// in resource tests.
+    pub fn stream_count(&self) -> usize {
+        self.streams.len()
+    }
 }
 
 /// Generates an HTTP/3 GREASE variable length integer.
@@ -7349,6 +7360,68 @@ mod tests {
         assert_eq!(s.poll_client(), Ok((stream, Event::Finished)));
         assert_eq!(s.poll_client(), Err(Error::Done));
     }
+
+    #[test]
+    fn h3_cleanup_streams() {
+        let mut buf = [0; 65535];
+        let mut s = Session::new().unwrap();
+        s.handshake().unwrap();
+
+        let init_streams_client = s.client.stream_count();
+        let init_streams_server = s.client.stream_count();
+
+        // Client sends HEADERS and doesn't fin
+        let (stream, req) = s.send_request(false).unwrap();
+
+        let ev_headers = Event::Headers {
+            list: req,
+            more_frames: true,
+        };
+
+        // Server receives headers.
+        assert_eq!(s.poll_server(), Ok((stream, ev_headers)));
+        assert_eq!(s.poll_server(), Err(Error::Done));
+
+        assert_eq!(s.client.stream_count(), init_streams_client + 1);
+        assert_eq!(s.server.stream_count(), init_streams_server + 1);
+
+        // Client sends body and fin
+        let body = s.send_body_client(stream, true).unwrap();
+
+        let mut recv_buf = vec![0; body.len()];
+
+        assert_eq!(s.poll_server(), Ok((stream, Event::Data)));
+        assert_eq!(s.recv_body_server(stream, &mut recv_buf), Ok(body.len()));
+
+        assert_eq!(s.poll_server(), Ok((stream, Event::Finished)));
+
+        assert_eq!(s.client.stream_count(), init_streams_client + 1);
+        assert_eq!(s.server.stream_count(), init_streams_server + 1);
+
+        // Server sends response and finishes the stream
+        let resp_headers = s.send_response(stream, false).unwrap();
+        let resp_body = s.send_body_server(stream, true).unwrap();
+
+        let mut resp_recv_buf = vec![0; resp_body.len()];
+
+        let ev_headers = Event::Headers {
+            list: resp_headers,
+            more_frames: true,
+        };
+
+        assert_eq!(s.poll_client(), Ok((stream, ev_headers)));
+        assert_eq!(s.poll_client(), Ok((stream, Event::Data)));
+        assert_eq!(s.recv_body_client(stream, &mut recv_buf), Ok(body.len()));
+
+        // The server stream should be gone now
+        assert_eq!(s.client.stream_count(), init_streams_client + 1);
+        assert_eq!(s.server.stream_count(), init_streams_server);
+
+        // Polling again should clean up the client
+        assert_eq!(s.poll_client(), Ok((stream, Event::Finished)));
+        assert_eq!(s.poll_client(), Err(Error::Done));
+        assert_eq!(s.client.stream_count(), init_streams_client);
+    }
 }
 
 #[cfg(feature = "ffi")]
diff --git a/quiche/src/stream/mod.rs b/quiche/src/stream/mod.rs
@@ -31,6 +31,7 @@ use std::sync::Arc;
 use std::collections::hash_map;
 use std::collections::HashMap;
 use std::collections::HashSet;
+use std::collections::VecDeque;
 
 use intrusive_collections::intrusive_adapter;
 use intrusive_collections::KeyAdapter;
@@ -98,6 +99,12 @@ pub struct StreamMap<F: BufFactory = DefaultBufFactory> {
     /// created streams, to prevent peers from re-creating them.
     collected: StreamIdHashSet,
 
+    /// Sequence of streams which have been completed, but not yet reported.
+    ///
+    /// This is used to track streams that a higher layer may want to know
+    /// have closed so that they can be cleaned up.
+    pending_collected: VecDeque<u64>,
+
     /// Peer's maximum bidirectional stream count limit.
     peer_max_streams_bidi: u64,
 
@@ -557,6 +564,7 @@ impl<F: BufFactory> StreamMap<F> {
         self.remove_flushable(&s.priority_key);
 
         self.collected.insert(stream_id);
+        self.pending_collected.push_front(stream_id);
     }
 
     /// Creates an iterator over streams that have outstanding data to read.
@@ -647,6 +655,12 @@ impl<F: BufFactory> StreamMap<F> {
                 self.local_max_streams_uni - self.peer_opened_streams_uni
     }
 
+    /// Return streams we have collected but not yet notified the transport
+    /// about.
+    pub fn collected_streams<'a>(&'a mut self) -> impl Iterator<Item = u64> + 'a {
+        self.pending_collected.drain(..)
+    }
+
     /// Returns the number of active streams in the map.
     #[cfg(test)]
     pub fn len(&self) -> usize {
