Skip to content

h3-quinn: stop_sending during an outstanding read is deferred until that read completes, so the peer gets STOP_SENDING with code 0 #361

Description

@vcarus

Summary

Since #331, h3_quinn::RecvStream::stop_sending applies the error code immediately only when no read is outstanding. While poll_data has a read_chunk future in flight, the code is parked in pending_stop and applied after that read resolves (h3-quinn/src/lib.rs, poll_data / stop_sending).

In the usual reason for calling stop_sending the peer is not sending anything (a cancelled request, a stalled body, an idle CONNECT tunnel), so that read never resolves. The stream is eventually dropped, and quinn::RecvStream::drop sends STOP_SENDING with error code 0 (quinn/src/recv_stream.rs: stop(0u32.into())). 0 is not a valid HTTP/3 error code (those start at 0x100), and the code the application asked for never reaches the wire.

How to hit it

  1. poll_data returns Pending once, i.e. any recv_data().await that is waiting on the peer.
  2. The application calls stop_sending(code) from another task, a select! arm or a timeout handler. self.stream is None, so the code goes into pending_stop.
  3. The stream is dropped. The peer sees STOP_SENDING(0x0).

Confirmed with probes: the branch storing pending_stop runs, and 0x0 is what goes out. This is exactly the cancel path the client docs describe ("Whenever the client wants to cancel this request, it can call stop_sending()", client::RequestStream), and the server-side way of closing the receive half of a CONNECT stream with a meaningful code.

Why it is structural

poll_data moves the quinn::RecvStream into a 'static ReusableBoxFuture because read_chunk borrows &mut self and quinn has no public poll-based zero-copy read (poll_read / poll_read_buf copy into a ReadBuf; poll_read_generic is private). So while a read is outstanding, nothing outside that future can call stop(). #331 fixed the panic from #330 by deferring the stop, and #357 worked around the same structure for recv_id(); the deferred stop is the remaining consequence.

For comparison, s2n-quic-h3's implementation of the same trait applies stop_sending immediately. A downstream user already patches around "quinn's default STOP_SENDING code 0" on their side (youyuanwu/tonic-h3#32) and attributes it to quinn; their Drop guard calls stop_sending, which is still deferred while a read is outstanding, so it does not reach the wire either.

Possible fixes

I am happy to send a PR for either; which one would you prefer?

  1. Inside h3-quinn only. Give the in-flight future a way to be told to stop: when self.stream is None, stop_sending sends the code into the future (a tokio::sync::oneshot or Notify kept next to read_chunk_fut); the future selects between read_chunk and that signal, and on the signal drops the read borrow, calls stream.stop(code) and hands the stream back. pending_stop goes away. Roughly 30 lines, no API change, no extra copy. Needs a decision on what poll_data returns after a stop (Ok(None) or StreamErrorIncoming::StreamTerminated).
  2. Ask quinn for a public poll_read_chunk (a thin wrapper over the private poll_read_generic) and drop the boxed future entirely: poll_data becomes a direct poll, stop_sending and recv_id become trivial, and the ReusableBoxFuture allocation disappears. Cleaner end state, but it spans two repositories and needs a quinn minimum-version bump.

Regression test either way: open a request, have the server send nothing, call stop_sending(code) on the client while recv_data is pending, drop the stream, and assert that the server's SendStream observes Stopped(code) rather than Stopped(0).

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions