Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

tokio: introduce io::Duplex #2661

Merged
merged 1 commit into from
Jul 22, 2020
Merged

tokio: introduce io::Duplex #2661

merged 1 commit into from
Jul 22, 2020

Conversation

seanmonstar
Copy link
Member

@seanmonstar seanmonstar commented Jul 15, 2020

DuplexStream provides a bidirectional type that can be used to simulate IO, but over an in-process piece of memory.

I've seen implementations of this rewritten over and over in codebases. They are frequently used in testing situations, where something like tokio_test::io wouldn't work (not trying to assert exact read and write calls, but just join a client and server in a single process).

@davidbarsky
Copy link
Member

Compilation failures aside, I'm a big +1 for this change. I've written one of these before and they're super handy.

One thing that could be useful (in the future) is exposing some mechanism to drop writes, pseudo-randomly or whatever.

@davidbarsky
Copy link
Member

Not sure how fancy you want to get Sean, but feel free to borrow as much of https://github.com/davidbarsky/stuff/blob/7cf9aa9684da4e3fffc7cdb5556bb65fb7d29af5/src/sim_stream.rs#L1 as you'd like.

@seanmonstar
Copy link
Member Author

Oh cool!

@davidbarsky
Copy link
Member

(I do think the naming on your PR is better, however.)

@carllerche
Copy link
Member

@seanmonstar
Copy link
Member Author

As an update, this should go in tokio::io, and the constructor should be a free function. SomethingStream seems better than Pipe, since they are bidirectional. Maybe MemStream and tokio::io::mem() are less stellar names, though.

@Darksonn Darksonn added A-tokio Area: The main tokio crate C-enhancement Category: A PR with an enhancement or bugfix. M-io Module: tokio/io labels Jul 16, 2020
tokio-util/src/io/mem.rs Outdated Show resolved Hide resolved
@davidbarsky
Copy link
Member

@carllerche

How does this relate to https://github.com/tokio-rs/tokio/blob/master/tokio-test/src/io.rs?

MemStream is complimentary to that. The existing mock is great for asserting that certain read/write actions happened, while MemStream-style APIs are really handy for testing interactions between a client and a server together, in memory. Without a MemStream-style API, people (including me and in linkerd2-proxy!) ended up using the OS' loopback interface and port 0 to grab an unused port.

Copy link
Member

@LucioFranco LucioFranco left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I didn't make my way through the whole thing since I have to head out but super excited for this!!!

tokio/src/io/util/mem.rs Outdated Show resolved Hide resolved
tokio/src/io/util/mem.rs Outdated Show resolved Hide resolved
tokio/src/io/util/mem.rs Show resolved Hide resolved
tokio/src/io/util/mem.rs Outdated Show resolved Hide resolved
tokio/src/io/util/mem.rs Outdated Show resolved Hide resolved
@seanmonstar
Copy link
Member Author

Another naming option is Duplex.

tokio::io::duplex() -> (Duplex, Duplex)

@Matthias247
Copy link
Contributor

My super old C# implementation of that was called BufferedPipe: https://github.com/Matthias247/http2dotnet/blob/master/Http2Tests/BufferedPipe.cs

Actually it was only a unidirectional stream, and I simply created 2 of those where it was required.
That could also be an option, since not all IO has bidirectional channels.

You can use this to model a unidirectional OS pipe, or a unidrectional QUIC channel, etc.

@seanmonstar seanmonstar changed the title tokio-util: introduce a MemStream tokio: introduce io::Duplex Jul 18, 2020
@seanmonstar
Copy link
Member Author

Update: renamed to DuplexStream, added a maximum buffer size, constructor is tokio::io::duplex(max_buf_size) -> (DuplexStream, DuplexStream).

I have no idea why CI is blowing up. The tests run locally for me, and if I make the changes it says in CI, it produces warnings locally. Wat?

@taiki-e
Copy link
Member

taiki-e commented Jul 18, 2020

CI failure is rust-lang/rust#73592. I'd suggest to add mut and #[allow(unused_mut)] to warned functions for compatibility with older compilers.

@seanmonstar seanmonstar force-pushed the util-memstream branch 2 times, most recently from ce321da to 8a9896d Compare July 21, 2020 15:54
@seanmonstar seanmonstar requested a review from a team July 21, 2020 16:43
tokio/src/io/util/mem.rs Outdated Show resolved Hide resolved
`duplex` returns a pair of connected `DuplexStream`s.

`DuplexStream` is a bidirectional type that can be used to simulate IO,
but over an in-process piece of memory.
Copy link
Member

@carllerche carllerche left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. Thanks.

@carllerche carllerche merged commit 0e090b7 into master Jul 22, 2020
@carllerche carllerche deleted the util-memstream branch July 22, 2020 22:07
@Darksonn Darksonn mentioned this pull request Nov 10, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-tokio Area: The main tokio crate C-enhancement Category: A PR with an enhancement or bugfix. M-io Module: tokio/io
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants