io: add a copy_bidirectional utility #3572
Conversation
|
@Darksonn could you take a look at the changes if you find time. Maybe rerunning the CI will fix the build errors, as I don't think this is a error on my side. |
|
I don't know when I'll have time to look over the changes, but I hit the button that reruns CI. |
|
The diff appears to have been messed up by the merge. Can you rebase the commits onto the current master and force push? |
|
@Darksonn thanks for the quick feedback, I have added the comments. CI is failing on some unrelated / unchanged files, which is weird as the master branch seems to pass CI. |
|
The CI stuff will be fixed by #3647. |
|
You should be able to merge in master or rebase again to fix the CI failure. |
|
Seems to be good now. |
| while self.pos < self.cap { | ||
| let me = &mut *self; | ||
| let i = ready!(writer.as_mut().poll_write(cx, &me.buf[me.pos..me.cap]))?; | ||
| if i == 0 { |
There was a problem hiding this comment.
I think it would make sense to try to read more data if the write returns Pending when me.cap < 2048.
There was a problem hiding this comment.
Will look into this
|
Interestingly enough, some colleagues and I were recently comparing TCP proxy implementations we wrote. My Rust version used a pair of It doesn't appear that I can make the buffer bigger (and this PR seems to keep the 2k buffer size). But I admit I'm a novice and may have missed a detail in the API. Is there a way to make the buffer bigger using this function? And if not, why was 2k chosen? |
|
No, there isn't any way of making the buffer bigger as it is now, but I wouldn't mind adding another method that takes a buffer size. |
I'm glad to add another method. but what about the function name? copy_with_buffer_size? any ideas? |
|
I guess I don't have a preference. |
|
Please open a new issue for discussing this. |
This is the same as `copy_bidirectional` but accepts the `a` -> `b` and `b` -> `a` buffers sizes. `copy_bidirectional` uses 8Kb buffers under the hood which isn't configurable. `copy_bidirectional_with_size` fixes it by allowing an user to set its own sizes for underlying buffers. Fixes: tokio-rs#6454. Refs: tokio-rs#3572.
This pull request is based on the work by @bdonlan i have implemented the requested changes of #3230.
Motivation
While there is a unidirectional copy utility currently, bidirectional copying between two streams (eg, proxying connections) is a bit tricky to do seamlessly and correctly due to the need to propagate EOFs and handle various error conditions.
Solution
This change adds a helper that can be used to, for example, glue together two TcpStreams and forward data across them.