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

WriteHalf<TcpStream>::shutdown won't shutdown the TcpStream #852

Closed
crab2313 opened this issue Jan 15, 2019 · 3 comments · Fixed by #1217
Closed

WriteHalf<TcpStream>::shutdown won't shutdown the TcpStream #852

crab2313 opened this issue Jan 15, 2019 · 3 comments · Fixed by #1217

Comments

@crab2313
Copy link

crab2313 commented Jan 15, 2019

Version

tokio 0.1.14

Platform

Linux archlinux 4.19.13-1-lts #1 SMP Sun Dec 30 07:38:47 CET 2018 x86_64 GNU/Linux

Subcrates

tokio-tcp

Description

I'm trying to implement a TCP proxy and I know there is an example. I noticed that calling the shutdown method of WriteHalf<TcpStream> does nothing. I find this after some research:

impl<'a> AsyncWrite for &'a TcpStream {
    fn shutdown(&mut self) -> Poll<(), io::Error> {
        Ok(().into())
    }

My question is that why this shutdown does nothing instead of calling TcpStream::shutdown(self, Shutdown::Write) since it allow us write something like this:

                    let (client_reader, client_writer) = client.split();
                    let (server_reader, server_writer) = server.split();

                    let client_to_server = copy(client_reader, server_writer)
                        .and_then(|(n, _, mut server_writer)| {
                            println!("copied {} bytes to server", n);
                            server_writer.shutdown()
                        });
                    let server_to_client = copy(server_reader, client_writer)
                        .and_then(|(n, _, mut client_writer)| {
                            println!("copied {} bytes to client", n);
                            client_writer.shutdown()
                        });
                    client_to_server.join(server_to_client)
@hawkw hawkw added the tokio-io label Jan 19, 2019
@ancwrd1
Copy link

ancwrd1 commented Feb 1, 2019

I have the same issue where it's not possible to shutdown the TcpStream after it has been split into read/write parts.

For now I have added a workaround in my code using wrapping struct but I think it should be fixed in the Tokio.

struct WrappedTcpStream(TcpStream);

impl Read for WrappedTcpStream {
    fn read(&mut self, buf: &mut [u8]) -> io::Result<usize> {
        self.0.read(buf)
    }
}

impl Write for WrappedTcpStream {
    fn write(&mut self, buf: &[u8]) -> io::Result<usize> {
        self.0.write(buf)
    }

    fn flush(&mut self) -> io::Result<()> {
        self.0.flush()
    }
}

impl AsyncRead for WrappedTcpStream {
    unsafe fn prepare_uninitialized_buffer(&self, buf: &mut [u8]) -> bool {
        self.0.prepare_uninitialized_buffer(buf)
    }

    fn read_buf<B: BufMut>(&mut self, buf: &mut B) -> Poll<usize, io::Error> {
        self.0.read_buf(buf)
    }
}

impl AsyncWrite for WrappedTcpStream {
    fn shutdown(&mut self) -> Poll<(), io::Error> {
        self.0.shutdown(Shutdown::Write)?;
        Ok(().into())
    }

    fn write_buf<B: Buf>(&mut self, buf: &mut B) -> Poll<usize, io::Error> {
        self.0.write_buf(buf)
    }
}

And then in the code:

   let (reader, writer) = WrappedTcpStream(stream).split();
   ....
   tokio::io::shutdown(writer)

@carllerche
Copy link
Member

Yes, I agree this is very annoying.

First, the AsyncWrite::shutdown name is bad. This will be changed when a breaking release happens.

The second problem is accessing TcpStream::shutdown once split. I am not aware of a great solution. Split uses a lock to coordinate. One option would be to add a custom split implementation directly on TcpStream, but that isn't great since TcpStream implements AsyncRead + AsyncWrite.

@bmwill
Copy link

bmwill commented May 2, 2019

This just bit me today (though I was just using a bare TcpStream, not wrapped in a WriteHalf). I ended up just wrapping the TcpStream object and hand implemented AsyncRead/AsyncWrite by delegating the AsyncRead/AsyncWrite to the TcpStream implementation except in the case of the shutdown method where I explicitly called TcpStream::shutdown(::std::net::Shutdown::Write) to force the write side to be shutdown.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants