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

Named Pipes: Support Message Read Mode #5349

Closed
mhils opened this issue Jan 5, 2023 · 2 comments · Fixed by #5350
Closed

Named Pipes: Support Message Read Mode #5349

mhils opened this issue Jan 5, 2023 · 2 comments · Fixed by #5350
Labels
A-tokio Area: The main tokio crate C-feature-request Category: A feature request. M-net Module: tokio/net

Comments

@mhils
Copy link
Contributor

mhils commented Jan 5, 2023

Context on Windows Named Pipes

Windows has message-based and byte stream-based pipes, but there also is an additional concept of read modes. From the docs:

The type mode of a pipe determines how data is written to a named pipe. Data can be transmitted through a named pipe as either a stream of bytes or as a stream of messages.
[...]
The read mode of a pipe determines how data is read from a named pipe. The pipe server specifies the initial read mode for a pipe handle when calling CreateNamedPipe. Data can be read in byte-read mode or message-read mode. A handle to a byte-type pipe can be in byte-read mode only. A handle to a message-type pipe can be in either byte-read or message-read mode. For a message-type pipe, the read mode can be different for server and client handles to the same pipe instance.

Is your feature request related to a problem? Please describe.

Tokio allows the creation of message-based named pipes using .pipe_mode, but it currently always opens them in byte read mode. In other words, when calling .read(), a "random" slice of bytes is returned (which may not be aligned with message boundaries). This was unexpected to me, I expected to read messages from my message pipe.

The same problem applies to NamedPipeClient instances, which always operate in byte read mode at the moment.

Describe the solution you'd like
When calling server_options.pipe_mode(PipeMode::Message), the pipe should not only be set to be a message type pipe, but it should also be opened in message read mode.

For NamedPipeClient, an additional .pipe_mode method should be added to the builder to switch to message read mode (equivalent to the server).

Describe alternatives you've considered
The first design decision here is whether tokio should expose Windows' distinction between pipe type and read mode. In other words, tokio could alternatively expose two distinct .pipe_mode and .read_mode builder methods. I personally feel that this comes at the risk of introducting subtle bugs (people will forget to also set .read_mode, which isn't immediately obvious).

The second design decision is whether ClientNamedPipe should 1) automatically query the pipe type and use the equivalent read mode, or 2) have an explicit .pipe_mode builder method. The explicit version seems more reasonable to me, but I don't have strong opinions on this either.

Additional context
I will follow up with a PR for the proposed changes momentarily. :)

@mhils mhils added A-tokio Area: The main tokio crate C-feature-request Category: A feature request. labels Jan 5, 2023
@Darksonn Darksonn added the M-net Module: tokio/net label Jan 5, 2023
@Darksonn
Copy link
Contributor

Darksonn commented Jan 5, 2023

If reading doesn't take the message boundaries into account, then I don't understand the difference between the pipe modes (when not setting the read mode).

@mhils
Copy link
Contributor Author

mhils commented Jan 5, 2023

Well yeah - I agree. :) It does take boundaries into account, but only if PIPE_READMODE_MESSAGE is set (which is what I propose to do).

I suppose for some weird reason the API also lets you consume a message-based pipe byte-by-byte, and that happens to be the default unless you explicitly specify the message read mode. 🤷

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-feature-request Category: A feature request. M-net Module: tokio/net
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants