quinn-rs / quinn

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

new_connection.uni_streams.next().await does not return until finish() is called #1365

Closed lso400 closed 2 years ago

lso400 commented 2 years ago

Client:

  let connecting = endpoint.connect(server_addr, SERVER_NAME).unwrap();
    let mut new_connection = connecting.await.unwrap();
    log::info!(
        "[client] connected: addr={}",
        new_connection.connection.remote_address()
    );
    loop {
        log::info!("checking if streamed is opened");
        if let Some(Ok(mut recv_stream)) = new_connection.uni_streams.next().await {
            // the below line never gets printed
            log::info!("stream opened from server");  
        }
    }

The client seems to get stuck in new_connection.uni_streams.next().await however the server has already opened a stream and started writing into it. The client is able to read the stream only when the server calls sender_stream.finish().await

djc commented 2 years ago

What version?

lso400 commented 2 years ago
quinn = "0.8.3"
rustls = { version = "*", features = ["dangerous_configuration", "quic"] }
rcgen = "0.9.2"
futures-util = { version = "0.3", default-features = false, features = ["sink", "std"] }
tokio = { version = "1.17.0",  features = ["full"] }
Ralith commented 2 years ago

Quinn always processes writes immediately; there is no equivalent to TCP's notorious "nagle's algorithm." In fact, call calling finish does is set a bit on the next outgoing frame, and force an empty frame if there isn't data queued. Hence, this should not happen. Can you demonstrate it with a minimal standalone reproduction?

lso400 commented 2 years ago

Thanks for clarifying. I tried to reproduce with a minimal setup but not successful, therefore spent some time to check what caused the difference. And I found that it might be related to crossbeam

if I'm sending data this way, data seems to be written ok but the client will get stuck on new_connection.uni_streams.next().await

loop {
  if let Ok(msg) = channel.try_recv() {
    sender_stream.write_all(msg).await.unwrap();
    log::info!("written into stream"); 
  }
}

It works if the if ... channel.try_recv() condition is removed. FYI channel is crossbeam_channel::Receiver<crate::IncomingMessage>

Is it incompatible or am I missing something? Thanks

Ralith commented 2 years ago

There is no compatibility issue. However, try_recv will immediately return None if there's nothing in the channel, which will cause that code to busy-loop, which will interfere with any other async task from functioning correctly on that thread.

lso400 commented 2 years ago

But the written into stream log line is printed, does it not mean the write_all has finished? I tried putting sender_stream.flush().await.unwrap() after the write and it does not help as well Also tried replacing try_recv with recv, no luck. Would you have any suggestion to get around this? Thanks.

Ralith commented 2 years ago

I suggest you fix the busy-loop in your code. async can't work as intended when a single task is consuming the entire CPU thread to do nothing.

Ralith commented 2 years ago

To be clear, blocking the thread with a blocking recv is almost as bad, since it too prevents other tasks from using the thread. You should use an async-capable channel that you can await on rather than spinning or blocking, if your application requires this pattern.

lso400 commented 2 years ago

Thanks a lot! Replacing crossbeam with async-channel solved my issue!