quinn-rs / quinn

Async-friendly QUIC implementation in Rust
Apache License 2.0
3.85k stars 394 forks source link

Return `UnknownStream` instead of panic when calling `finish()` on an already closed `SendStream` #1569

Closed EAimTY closed 1 year ago

EAimTY commented 1 year ago

I think the behavior should be consist of reset(). The panic is caused by

https://github.com/quinn-rs/quinn/blob/4b309cd46b5a68c96c105b00dfbc65216523677f/quinn/src/send_stream.rs#L143C11-L160

which polls and unwraps a used oneshot receiver.

I created a wrapper over a pair of SendStream and RecvStream to use tokio::io::copy_bidirectional on them. Since tokio::io::copy_bidirectional returns error immediately after encountering error on any direction without gracefully shutting down the other direction, I sometimes needs to manually close the other one. However, there is no way to distinguish which one should I close, it is required to explicitly close both one. Unfortunately, the SendStream may already been closed by tokio::io::copy_bidirectional, and calling close on it multiple times would cause panic. Additionally, the panicked thread holds the MutexGuard, which causes the lock poisoning and crashes the whole program.

djc commented 1 year ago

Sounds good to me, want to submit a PR?

Ralith commented 1 year ago

This was fixed in https://github.com/quinn-rs/quinn/pull/1588.