-
Notifications
You must be signed in to change notification settings - Fork 292
Description
Hi! I found RecvStream::read_to_end confusing. Copying a thread of conversation we had on discord (starting here) to elaborate:
me
hi! back with more questions
how would one keep the connection open and send stuff back and forth? I've realized that if I don't close the stream, I get a ping from the client, the server notices the connection and
accept_bi
's fine, but then everything hangs from there.I've looked at the docs for
Connection::open_bi
and the send/recv streams, and they don't seem to mention how.
...
matheus23
So what you did is remove the SendStream::finish calls (which makes sense if you want to be able to ping on the same stream again), but you then keep the RecvStream::read_to_end calls, specifically the to_end variants. This means that call will hang because it's waiting for the SendStream on the other side to finish, which it never does.
If you want to fix this, you need to either call read_exact or something that isn't reading to end. And you'll need to do your own framing. In this case you'll know that each ping is exactly 4 bytes, so you could call read_exact(&mut buf) (with let mut buf = [0u8; 4];). In other cases, when you don't know the framing, then you'll need to actually do "real framing" where you prefix each message you send with its size encoded in some known format (e.g. always encoded as big-endian u32s).
me
ahhhh I see! I was a bit confused by the docs on that because I wasn't sure what "read all remaining data" meant. I somewhat remembered reading to end in stdio and in C, where it waits for EOF, but that didnt seem quite right for binary data. I ended up not thinking about it further, whoops :p
Either way, it would be helpful to have docs on what exactly defines all remaining—https://doc.rust-lang.org/std/io/trait.Read.html#method.read_to_end specifies EOF, and i think it would be nice to have a corresponding mention for Send::finish in Recv::read to end
Getting this tweaked in upstream quinn would also be neat.
Metadata
Metadata
Assignees
Labels
Type
Projects
Status