Connection::stream_send() reports Err::Done for finished streams

[bwoebi]

This case occurs with streams closed with STOP_SENDING, then a fin bit being received too, then the data being read via Connection::stream_recv().

In high-level terms:

server opens stream - sends some data
client reads some data and writes data
server reads that data, sends STOP_SENDING, and writes some data, then sends the fin bit in a separate empty message of length 0
client sees stream as readable, then reads the data
client tries to write some data ... gets Err::Done from stream_send().

As far as I gathered from the documentation, Err::Done is supposed to be returned when no data could be written because of no capacity and Err::StreamStopped returned when the stream has been closed by the peer.

In this specific case though, the stream seems to already have been freed by quiche, thus StreamMap::get_or_create() returns Err::Done, which Connection::stream_send() simply forwards.

I would expect an Err::StreamStopped, or at least an Err::InvalidStreamState (signaling that a stream with that id could not be created, in that case, obviously, because the stream id is not matching what's allowed for the client/server).

Currently I'm working around it with an extra check of Connection::stream_writable() for Err::InvalidStreamState whenever Err::Done is reported, but that seems unnecessary/wrong to me, why would that be needed?

In case this is the expected behaviour though, I would like to ask for a note in the documentation to check stream_writable() explicitly if Err::Done is returned.

[LPardue]

Thanks for the detailed report. Thatspecific sequence would lead to the stream being collected, and hence always get_or_create will return Done.

For stream_recv we return a InvalidStreamState if the stream doesn't exist https://github.com/cloudflare/quiche/blob/d4e5ed6affb35fdeba10731eaaa08b0c84965901/quiche/src/lib.rs#L4476

We could do similar for stream_send, capture the Done and convert it to InvalidStreamState at https://github.com/cloudflare/quiche/blob/d4e5ed6affb35fdeba10731eaaa08b0c84965901/quiche/src/lib.rs#L4619. Using StopSending for the collected streams seems like it would be confusing two different stream states.

[bwoebi]

Yes, an InvalidStreamState would be probably most appropriate here with the current architecture.

It would be great if there were a solution for #1299 too, but I suppose quiche has no mode where we can instruct it to explicitly keep streams alive until they are manually collected with a method call (Connection::stream_free()) or such.

[LPardue]

Oh thanks for the reminder on that. I guess if we're considering g changing the API, we might want to follow through with a solution that can satisfy both.