Skip to content

fix: flush data buffered by send_data before the next write and before finish - #360

Open
vcarus wants to merge 1 commit into
hyperium:masterfrom
vcarus:fix/flush-buffered-data-before-finish
Open

fix: flush data buffered by send_data before the next write and before finish#360
vcarus wants to merge 1 commit into
hyperium:masterfrom
vcarus:fix/flush-buffered-data-before-finish

Conversation

@vcarus

@vcarus vcarus commented Aug 20, 2026

Copy link
Copy Markdown

What happens

stream::write in h3 is send_data followed by poll_ready. That future is not cancel-safe: when it is dropped between the two (a tokio::time::timeout around RequestStream::send_data, a select! branch losing, an application deadline), the transport keeps the rest of the frame in its writing buffer.

From there two things go wrong:

  1. The connection dies. The next send_data on that stream finds writing.is_some(). h3-quinn treats that as a misuse of the trait (// This can only happen if the traits are misused by h3 itself) and returns ConnectionErrorIncoming::InternalError, which handle_quic_stream_error turns into closing the whole QUIC connection with H3_INTERNAL_ERROR. Every other request multiplexed on that connection is gone. On the first request stream of a connection a plain finish() is enough to hit this, because finish writes a grease frame first; on later streams any further send_data or send_trailers does it.
  2. The peer sees a truncated frame. h3-quinn's poll_finish calls quinn::SendStream::finish() without flushing writing, so FIN goes out while the tail of the last DATA frame is still buffered. This one does not even need a cancellation: send_data + poll_finish with no poll_ready in between already loses the data.

Reproduction for (1), as in the new test: send a response body larger than the peer's stream receive window, poll the send_data future once, drop it, then call finish(). Before this change the server gets ConnectionError(Remote(InternalError("internal error in the http stack"))) and the client sees ApplicationClose(H3_INTERNAL_ERROR).

The change

  • h3/src/stream.rs: write polls poll_ready before send_data as well as after. The leading poll is a no-op when nothing is buffered and otherwise flushes the leftover of a cancelled write, so send_data is never called on a stream that is not ready.
  • h3-quinn/src/lib.rs: poll_finish flushes via poll_ready before finish(). This matches what s2n-quic-h3's implementation of the same trait already does (ready!(self.poll_ready(cx))?; self.stream.finish()); the two send_data guards are word-for-word identical, h3-quinn just lacked the flushing half.

Two regression tests, one per change; each fails without its change and passes with it (finish_after_cancelled_send_data_keeps_connection_open in tests/request.rs, h3_quinn_finish_flushes_buffered_data in tests/connection.rs). cargo test passes for the three feature sets CI runs.

Relation to #78 / #81

#81 swapped the order to poll_ready then send_data and dropped the trailing poll_ready; that stalled transfers because h3-quinn only writes on poll_ready, so data sat in the buffer until the next call. This PR keeps the trailing poll_ready, so a write still completes within the call, and only adds the leading one. It does not change the trait contract or any public API.

Found while running a MASQUE (CONNECT-UDP) proxy on h3, where a client that stops reading one tunnel used to take down every other tunnel on the connection.

…e finish

h3's `stream::write` is `send_data` followed by `poll_ready`. When that future is dropped between the two, for example by `tokio::time::timeout` or `select!`, the transport keeps the rest of the frame in its buffer. The next `send_data` on that stream then finds the stream not ready; h3-quinn treats that as a misuse of the trait and answers with `ConnectionErrorIncoming::InternalError`, which closes the whole QUIC connection with H3_INTERNAL_ERROR. On the first request stream of a connection a plain `finish()` is enough to get there, because it writes a grease frame first.

Independently, h3-quinn's `poll_finish` sent FIN without flushing that buffer, so the peer saw a truncated frame.

`stream::write` now polls `poll_ready` before `send_data` (a no-op when nothing is buffered) and keeps the trailing `poll_ready`, so a write still completes within the call. h3-quinn's `poll_finish` flushes before `finish`, matching s2n-quic-h3's implementation of the same trait.

Both paths get a regression test that fails without the corresponding change.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant