uni stream can't be cleaned up if closed while data in flight

[Haplo79]

Scenario (run on loopback):

Server

• A server limited to one uni-stream (max_concurrent_uni_streams(1u32.into()) listens for incoming uni streams.
• When it receives one, it spawns this tokio task to simply read one byte and then exit:

async fn drain_uni_stream(mut recv_stream: quinn::RecvStream) -> anyhow::Result<()> {
    let result = recv_stream.read_u8().await;
    if let Ok(byte) = result {
        println!("got u8: {}", byte);
    } else {
        println!("error: {:?}", result);
    }

    println!("leaving");
    Ok(())
}

Client

• A client limited to one uni-stream (max_concurrent_uni_streams(1u32.into()) tries to open uni-streams to the server in a loop:

  
  async fn create_loop(connection: quinn::Connection) -> anyhow::Result<()> {
  loop {
      _ = create_stream_and_write(connection.clone())
          .await
          .map_err(|e| println!("write to stream {:?}", e));
  
      tokio::time::sleep(tokio::time::Duration::from_secs(1)).await;
  }
  }

async fn create_stream_and_write(connection: quinn::Connection) -> anyhow::Result<()> { println!("creating new stream"); let mut uni = connection.open_uni().await?; println!("new stream created");

let result = writes(&mut uni).await;

println!("leaving");
result

}

async fn writes(uni: &mut quinn::SendStream) -> anyhow::Result<()> { for _ in 0..500 { println!("writing"); uni.write_u8(245).await.context("write failure")?; }

Ok(())

}

The client output is this:
``` text
creating new stream
new stream created
writing
writing
... about 15 more "writing"
writing
leaving
write to stream write failure

Caused by:
    sending stopped by peer: error 0
creating new stream

Notes:

• The client is writing a lot more than the server is reading, that is purposeful to cause a repro of this issue.
• The client starts writing, and then about 20 writes in, gets an error that the server has stopped receiving. So it exits the writes() function and goes through the loop in create_loop() to call create_stream_and_write() again.
• The call to let mut uni = connection.open_uni().await?; in create_stream_and_write() never returns, from what I can tell this is because the client thinks that the server still has a uni_stream consumed, and isn't allowed any more.
• That would be fine, except I don't see any way to get this stream "released". Rearranging writes() so that I can call uni.reset() or uni.finish() when I see an error makes no difference that I can tell.
• This looks to be a race of some sort. If I reduce the number of iterations of the for loop in writes() to 5, then there is no more hanging on creating a new stream. The client just happily loops through creating and destroying new streams.
• My wild guess is that if the receiver of a uni stream calls stop() (which is implied by dropping the RecvStream, near as I can tell) while enough traffic is still in flight from the client, the server isn't able to fully clean up the state for this stream - maybe because it never acks everything the client sent it and so the client doesn't believe the stream is closed..?
• Each iteration on the client "leaks" one stream, so having the server/client set to 1 stream reproduces the problem faster, but isn't required in order to reproduce, it will just take longer.

[Ralith]

Thanks for the report! I agree that this looks fishy. New flow control credit should be issued after both peers drop their handles to the stream. Can you share a complete runnable repro?

[Haplo79]

@Ralith Let me know if the attached source counts as a runnable repro. I'm not quite sure the best way to share that in a forum like this, maybe I should create a repo?

quinn_issue.zip

[Ralith]

That's perfect, thanks! Behavior is as described. I'll investigate this week.

[Ralith]

This turned out to be pretty simple. The I/O on the connection is not specifically significant: the issue is that we don't immediately update stream ID flow control when a stream that has been stopped is reset. Do you want a backport for 0.8?

[Haplo79]

As to backport - am prepared to jump on .9 as soon as its released, and that would probably be the best way to get this fix. If .9 is more than a couple of weeks away, I certainly would not refuse a backport if its easy. :) Thanks.