search

quinn-rs / quinn

Async-friendly QUIC implementation in Rust
Apache License 2.0
3.57k stars 364 forks source link

Resolve stopped/received_reset futures on lost connections #1886

Closed Ralith closed 3 weeks ago

Ralith commented 3 weeks ago

If a connection is lost before a stream becomes closed, these would otherwise never complete.

djc commented 3 weeks ago

Stared at this a little bit in the afternoon but got caught up wondering about why we shouldn't yield the connection errors on any of the other code paths. Is that correct? Why?

djc commented 3 weeks ago

(In other words I agree that we should not yield Pending on these paths but wasn't sure we should still yield Ok(None) if conn.error is Some.)

Ralith commented 3 weeks ago

got caught up wondering about why we shouldn't yield the connection errors on any of the other code paths. Is that correct? Why?

This is intended, though there's room for debate. My general philosophy for accessing state on closed connections is that information that was available immediately before the connection became closed should not be lost/inaccessible after it becomes closed. Hence, if a receive stream is already finished or reset, or a send stream is fully ACKed or stopped, then we continue to expose that information to the application even if the connection has since been lost.

See also RecvStream::poll_read_generic, which only checks conn.error when no further data can be read, allowing previously-received stream data to be drained even after a connection is lost.

djc commented 3 weeks ago

Fair!