h3-quinn: `stop_sending` during an outstanding read is deferred until that read completes, so the peer gets STOP_SENDING with code 0
Nobody has claimed this yet.
- Dominant language
- Rust
- Stars
- 890
- Forks
- 136
- Avg merge
- 14d 20h
- Merged PRs (30d)
- 2
Description
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
poll_datareturnsPendingonce, i.e. anyrecv_data().awaitthat is waiting on the peer.- The application calls
stop_sending(code)from another task, aselect!arm or a timeout handler.self.streamisNone, so the code goes intopending_stop. - 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?
- Inside h3-quinn only. Give the in-flight future a way to be told to stop: when
self.streamisNone,stop_sendingsends the code into the future (atokio::sync::oneshotorNotifykept next toread_chunk_fut); the future selects betweenread_chunkand that signal, and on the signal drops the read borrow, callsstream.stop(code)and hands the stream back.pending_stopgoes away. Roughly 30 lines, no API change, no extra copy. Needs a decision on whatpoll_datareturns after a stop (Ok(None)orStreamErrorIncoming::StreamTerminated). - Ask quinn for a public
poll_read_chunk(a thin wrapper over the privatepoll_read_generic) and drop the boxed future entirely:poll_databecomes a direct poll,stop_sendingandrecv_idbecome trivial, and theReusableBoxFutureallocation 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).
Contributor guide
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- Open a pull request that references the issue number.
Research direction
Start in h3-quinn/src/lib.rs, reading RecvStream::poll_data and stop_sending, then inspect quinn/src/recv_stream.rs for the drop behavior. Reproduce a pending recv_data followed by stop_sending and stream drop, and add a regression test that verifies the peer observes Stopped(code), not Stopped(0); resolve which proposed fix and post-stop result to use.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- rust
- Domain
- networking
- Issue type
- Bug
- Difficulty
- 4/5
- Estimated time
- 3-5 days
- Activity status
- Active
- Clarity
- Mostly clear
- Newbie friendliness
- 48/100