Reading from named pipe seems limited to 4096 bytes

[LeonarddeR]

Version
tokio v1.23.0

Platform
Windows 11 64-bit

Description
I'm using Tokio as a named pipe server. However, when reading data from a pipe, I"m only getting chunks with a max of 4096 bytes, even though my vector has a higher capacity.

I tried this code:

        ASYNC_RUNTIME.spawn(async move {
            let mut first_pipe_instance = true;
            loop {
                trace!("Creating pipe server with address {}", pipe_addr);
                let server = ServerOptions::new()
                    .first_pipe_instance(first_pipe_instance)
                    .max_instances(1)
                    .create(&pipe_addr)
                    .unwrap();
                first_pipe_instance = false;
                trace!("Initiate connection to pipe client");
                server.connect().await.unwrap();
                let (mut server_reader, server_writer) = split(server);
                trace!("Pipe client connected. Initiating pipe_reader loop");
                loop {
                    let mut buf = Vec::with_capacity(1024 * 64);
                    match server_reader.read_buf(&mut buf).await {
                        Ok(0) => {
                            info!("Received 0 bytes, pipe closed by client");
                            break;
                        }
                        Ok(n) => {
                            trace!("read {} bytes", n);
                        }
                        Err(e) if e.kind() == WouldBlock => {
                            warn!("Reading pipe would block: {}", e);
                            continue;
                        }
                        Err(e) => {
                            error!("Error reading from pipe server: {}", e);
                            break;
                        }
                    }
                }
            }
        })

I expected to see this happen: When connecting to this pipe from a client and sending a message of 8192 bytes, I'd expect a message of 8192 bytes coming in at the sever's end.

Instead, this happened: The message came in as two chunks of 4096 bytes in size. It seems like there is a hard limit of 4096 bytes somewhere.

[Darksonn]

Seems like this one:

https://github.com/tokio-rs/mio/blob/aaf712e10cd33da4d67b590bad0b7427311d0dcb/src/sys/windows/named_pipe.rs#L805-L807

Thoughts @Thomasdezeeuw ?

[Thomasdezeeuw]

Yes, @carllerche wrote this code I believe. It's already a know issue, see tokio-rs/mio#1582 and tokio-rs/mio#1608, but we don't really have a good solution for it at the moment.

[Thomasdezeeuw]

Maybe we can use the length/capacity of the buffer passed in initialising or resizing the buffer. The pr I send with an additional method is not great as then we're stuck with exposing that implementation details (no other types in Mio use a buffer pool).

[LeonarddeR]

I guess it makes no sense to tie the buffer size to the buffer size of the named pipe itself? May be a dumb question, but isn't it possible to allocated the buffer as needed?

[Thomasdezeeuw]

I guess it makes no sense to tie the buffer size to the buffer size of the named pipe itself?

I'm not familiar enough with Window's named pipe to determine if this a good idea or not.

May be a dumb question, but isn't it possible to allocated the buffer as needed?

That's what we're already doing right now. The problem is that we pass ownership of the buffer to the Windows kernel while scheduling a read. But that doesn't work with the io::Read/io::Write traits while only get a mutable reference. Hence we we allocate another buffer to pass to the kernel only to copy it's contents to the user's buffer on the next read call. Like I suggested in #5307 (comment), we could perhaps make this Mio internal buffer have the same size as the passed buffer. But I'm not sure what influence that has on other operations.

[LeonarddeR]

I guess it makes no sense to tie the buffer size to the buffer size of the named pipe itself?

I'm not familiar enough with Window's named pipe to determine if this a good idea or not.

I will try and perform some tests with this.

May be a dumb question, but isn't it possible to allocate the buffer as needed?

That's what we're already doing right now. The problem is that we pass ownership of the buffer to the Windows kernel while scheduling a read. But that doesn't work with the io::Read/io::Write traits while only get a mutable reference. Hence we we allocate another buffer to pass to the kernel only to copy it's contents to the user's buffer on the next read call. Like I suggested in #5307 (comment), we could perhaps make this Mio internal buffer have the same size as the passed buffer. But I'm not sure what influence that has on other operations.

I can also try to create/test a proof of concept for this if you'd like that.

[Thomasdezeeuw]

I will try and perform some tests with this.

👍

I can also try to create/test a proof of concept for this if you'd like that.

That sounds good. I think the simplest version would be to add a capacity argument to get_buffer and only use that many bytes in the calls to read. This will get a little awkward if a different size buffer is used in the next call to read (where the actual bytes are copied), but we'll see.

[LeonarddeR]

I will try and perform some tests with this.

👍

First tests with a buffer of two meant significant performance degradation, but I need to dive deeper into the implications of smaller buffers.

I can also try to create/test a proof of concept for this if you'd like that.

That sounds good. I think the simplest version would be to add a capacity argument to get_buffer and only use that many bytes in the calls to read. This will get a little awkward if a different size buffer is used in the next call to read (where the actual bytes are copied), but we'll see.

As that is indeed awkward, how about an approach where you can provide a size but the actual size is max of the given size and the last size used? This means that buffers can grow over time if the caller increases them, but they will never become smaller.

[LeonarddeR]

Here is a proof of concept commit, I'll see how it behaves later this week:
LeonarddeR/mio@4ad0aa4

Update: Never mind, this doesn't fix anything at all, we're still getting chunks of 4096 bytes at least in my example code.
It might be better to base of tokio-rs/mio#1608 and update the buffer size when initiating a new read.
I'm also slightly worried in that the same buffer seems to be used for writes, but I could be mistaken.