quinn-rs / quinn

Async-friendly QUIC implementation in Rust
Apache License 2.0
3.76k stars 380 forks source link

Add method Connection::finish #1542

Closed kpp closed 11 months ago

kpp commented 1 year ago

There are methods SendStream::{reset, finish} but only Connection::close.

It would be nice to have a method Connection::finish to gracefully shut down the connection.

I can do that for you if want.

djc commented 1 year ago

What, to you, are important aspects in gracefully shutting down the connection (that close() does't give you today)?

kpp commented 1 year ago

close() docs:

Pending operations will fail immediately with ConnectionError::LocallyClosed. Delivery of data on unfinished streams is not guaranteed, so the application must call this only when all important communications have been completed, e.g. by calling finish on outstanding SendStreams and waiting for the resulting futures to complete.

So an important aspect is delivery of data on unfinished streams.

Ralith commented 1 year ago

Relying on finish for application-layer semantics is a bit dicey; see e.g. https://github.com/quinn-rs/quinn/issues/1487#issuecomment-1409980709. Usually it makes more sense to reason about connection state at a higher level, and only close a connection that you are logically done with. In that light, can you elaborate on the use case for this?

djc commented 11 months ago

Closing for lack of follow-up. Feel free to reopen if there is still something here that should be pursued.