fix(http3): only add to stream_has_pending_data on has_data_to_send

`<SendMessage as Sendstream>::send_data` attempts to send a slice of data down
into the QUIC layer, more specifically `neqo_transport::Connection::stream_send_atomic`.

While it attempts to send any existing buffered data at the http3 layer first,
it does not itself fill the http3 layer buffer, but instead only sends data, if
the lower QUIC layer has capacity, i.e. only if it can send the data down to the
QUIC layer right away.

https://github.com/mozilla/neqo/blob/5dfe106669ccb695187511305c21b8e8a8775e91/neqo-http3/src/send_message.rs#L168-L221

`<SendMessage as Sendstream>::send_data` is called via
`Http3ServerHandler::send_data`. The wrapper first marks the stream as
`stream_has_pending_data`, marks itself as `needs_processing` and then calls
down into `<SendMessage as Sendstream>::send_data`.

https://github.com/mozilla/neqo/blob/5dfe106669ccb695187511305c21b8e8a8775e91/neqo-http3/src/connection_server.rs#L51-L74

Thus the latter always marks the former as `stream_has_pending_data` even though
the former never writes into the buffer and thus might actually not have pending
data.

Why is this problematic?

1. Say that the buffer of the `BufferedStream` of `SendMessage` is empty.

2. Say that the user attempts to write data via `Http3ServerHandler::send_data`.
Despite not filling the http3 layer buffer, the stream is marked as
`stream_has_pending_data`.

3. Say that next the user calls `Http3Server::process`, which will call
`Http3Server::process_http3`, which will call
`Http3ServerHandler::process_http3`, which will call
`Http3Connection::process_sending`, which will call `Http3Connection::send_non_control_streams`.

`Http3Connection::send_non_control_streams` will attempt to flush all http3
layer buffers of streams marked via `stream_has_pending_data`, e.g. the stream
from step (2). Thus it will call `<SendMessage as SendStream>::send` (note
`send` and not the previous `send_data`). This function will attempt the
stream's http3 layer buffer. In the case where the http3 layer stream buffer is
empty, it will enqueue a `DataWritable` event for the user. Given that the
buffer of our stream is empty (see (1)) such `DataWritable` event is always emitted.

https://github.com/mozilla/neqo/blob/5dfe106669ccb695187511305c21b8e8a8775e91/neqo-http3/src/send_message.rs#L236-L264

The user, on receiving the `DataWritable` event will attempt to write to it via
`Http3ServerHandler::send_data`, back to step (2), thus closing the busy loop.

How to break the loop?

This commit adds an additional check to the `stream_has_pending_data` function to
ensure it indeed does have pending data. This breaks the above busy loop. In
addition, it renames the function to `stream_might_have_pending_data`.

neqo-http3/src/connection.rs

@@ -386,10 +386,16 @@ impl Http3Connection {
         Ok(())
     }
 
-    /// Inform a `HttpConnection` that a stream has data to send and that `send` should be called
-    /// for the stream.
-    pub fn stream_has_pending_data(&mut self, stream_id: StreamId) {
-        self.streams_with_pending_data.insert(stream_id);
+    /// Inform a [`Http3Connection`] that a stream might have data to send in
+    /// case `send` should be called for the stream.
+    pub fn stream_might_have_pending_data(&mut self, stream_id: StreamId) {
+        if self
+            .send_streams
+            .get(&stream_id)
+            .map_or(false, |s| s.has_data_to_send())
+        {
+            self.streams_with_pending_data.insert(stream_id);
+        }
     }
 
     /// Return true if there is a stream that needs to send data.

neqo-http3/src/connection_server.rs

@@ -64,13 +64,15 @@ impl Http3ServerHandler {
         data: &[u8],
         conn: &mut Connection,
     ) -> Res<usize> {
-        self.base_handler.stream_has_pending_data(stream_id);
-        self.needs_processing = true;
-        self.base_handler
+        let n = self
+            .base_handler
             .send_streams
             .get_mut(&stream_id)
             .ok_or(Error::InvalidStreamId)?
-            .send_data(conn, data)
+            .send_data(conn, data)?;
+        self.base_handler.stream_might_have_pending_data(stream_id);
+        self.needs_processing = true;
+        Ok(n)
     }
 
     /// Supply response heeaders for a request.
@@ -87,7 +89,7 @@ impl Http3ServerHandler {
             .http_stream()
             .ok_or(Error::InvalidStreamId)?
             .send_headers(headers, conn)?;
-        self.base_handler.stream_has_pending_data(stream_id);
+        self.base_handler.stream_might_have_pending_data(stream_id);
         self.needs_processing = true;
         Ok(())
     }
@@ -100,7 +102,7 @@ impl Http3ServerHandler {
     pub fn stream_close_send(&mut self, stream_id: StreamId, conn: &mut Connection) -> Res<()> {
         qdebug!([self], "Close sending side stream={}.", stream_id);
         self.base_handler.stream_close_send(conn, stream_id)?;
-        self.base_handler.stream_has_pending_data(stream_id);
+        self.base_handler.stream_might_have_pending_data(stream_id);
         self.needs_processing = true;
         Ok(())
     }