h3
h3 copied to clipboard
hyperium
Fix write to check poll_ready before sending data
The current Quinn implementation doesn't actually send the data on the stream; it just buffers it until a poll_ready is called again. So that would need to change as well.
https://github.com/hyperium/h3/blob/4ccd1a282ae751e24a44283018e149544cd2500e/h3-quinn/src/lib.rs#L421-L425
https://github.com/hyperium/h3/blob/4ccd1a282ae751e24a44283018e149544cd2500e/h3-quinn/src/lib.rs#L382-L385
Oops, approved this a bit too quickly... I looks like this swap makes all the transfers fail. I vaguely remember having this problem. @camshaft suggestion might help.
Also, do you think the transfer should be initiated at the first send_data() call ?
Like mentioned in the issue, it would be most obvious to behave like the Sink trait:
poll_ready
This method must be called and return Poll::Ready(Ok(())) prior to each call to start_send.
This method returns Poll::Ready once the underlying sink is ready to receive data.
start_send
Begin the process of sending a value to the sink. Each call to this function must be preceded by a successful call to poll_ready which returned Poll::Ready(Ok(())).
As the name suggests, this method only begins the process of sending the item. If the sink employs buffering, the item isn’t fully processed until the buffer is fully flushed. Since sinks are designed to work with asynchronous I/O, the process of actually writing out the data to an underlying object takes place asynchronously. You must use poll_flush or poll_close in order to guarantee completion of a send.
Right, maybe I should have read the issue beforehand :sweat_smile: . Thanks!
While we're at it, we might want to ditch AsyncWrite by doing what Ralith suggested in this issue:
quinn could also be modified to expose raw
poll_*style methods alongside important futures, if a better approach is required in the mean time. That's assuming it's not practical to modify h3 to push the pinning/lifetime issues downstream.
Or maybe implement Sinkdirectly in quinn and use it.
Yeah AsyncWrite isn't the most efficient way of transferring data, especially when the data is already a Bytes. With s2n-quic we've exported poll functions for all of the stream operations. It leads to a really clean implementation of the h3 traits.