How to store TcpStream with BufReader and BufWriter in a data structure

Viewed 3660

I wanted a TcpStream shared by both a BufReader and a BufWriter, I found a solution in:
If BufReader takes ownership of a stream, how can I read and write lines on it?

Now I want it in its own data structure, but I only got a partial answer from:
Why can't I store a value and a reference to that value in the same struct?

The desired implementation is violating ownership rules.

use std::io::{BufReader, BufWriter};
use std::net::TcpStream;

pub struct BufTcpStream<'a> {
    _socket: TcpStream,
    input:  BufReader<&'a TcpStream>;
    output: BufWriter<&'a TcpStream>;
}

impl<'a> BufTcpStream<'a> {
    pub fn new(socket: TcpStream) -> Self {
        Self{
            input : BufReader::new(&socket),
            output: BufWriter::new(&socket),
            _socket: socket,//                 <-- MOVE OF BORROWED VALUE HERE
        }
    }
}

To solve this problem, I had to ensure the TcpStream references will stay valid during all the structure lifetime, I used a Pin<Box<TcpStream>> to ensure it.

But the compiler still complain about the move of the borrowed value socket. To remove this barrier I used std::meme::transmute().

Now, what i want to know is:

Is this implementation safe?

use std::io::{BufReader, BufWriter};
use std::net::TcpStream;
use std::pin::Pin;

pub struct BufTcpStream<'a> {
    _socket: Pin<Box<TcpStream>>,
    input : BufReader<&'a TcpStream>,
    output: BufWriter<&'a TcpStream>,
}

impl<'a> BufTcpStream<'a> {
    pub fn new(socket: TcpStream) -> Self {
        let pin = Box::pin(socket);
        unsafe {
            Self{
                input : BufReader::new(std::mem::transmute(&*pin)),
                output: BufWriter::new(std::mem::transmute(&*pin)),
                _socket: pin,
            }
        }
    }
    pub fn reach(&mut self) -> (
        &mut BufReader<&'a TcpStream>,
        &mut BufWriter<&'a TcpStream>
    ) {
        (&mut self.input, &mut self.output)
    }
}
2 Answers

Use TcpStream::try_clone to get a second stream:

The returned TcpStream is a reference to the same stream that this object references. Both handles will read and write the same stream of data, and options set on one stream will be propagated to the other stream.

You can then wrap one in a reader and one in a writer:

use std::{
    io::{self, BufReader, BufWriter},
    net::TcpStream,
};

struct BufTcpStream {
    input: BufReader<TcpStream>,
    output: BufWriter<TcpStream>,
}

impl BufTcpStream {
    fn new(stream: TcpStream) -> io::Result<Self> {
        let input = BufReader::new(stream.try_clone()?);
        let output = BufWriter::new(stream);

        Ok(Self { input, output })
    }
}

See also:

No. This implementation is not safe. For two reasons.

1) A reference to the TcpStream can live longer than the structure itself

let bad_ref = {
    let buffer = BufTcpStream::new(...);
    let (i, o) = buffer.reach();
    *i.get_ref()
};// <-- buffer is dropped here
// bad_ref is now an invalid ref but the compiler won't see it
bad_ref.write(b"hello world").unwrap();

The *i.get_ref() is of type &'a TcpStream, but the &'a lifetime is never inferred by the compiler, so it does not take it in account.

It can be fixed in the reach() method, by linking the lifetime of self to the returned references:

fn reach<'b>(&'b mut self) -> (
    &mut BufReader<&'b TcpStream>,
    &mut BufWriter<&'b TcpStream>
) {
    (&mut self.input, &mut self.output)
}


2) The TcpStream is dropped before BufReader and BufWriter

pub struct BufTcpStream<'a> {
    _socket: Pin<Box<TcpStream>>,//       First to be dropped   |
    input : BufReader<&'a TcpStream>,//   Second to be dropped  |
    output: BufWriter<&'a TcpStream>,//   Last to be dropped    V
}

It can be fixed by placing _socket as the last element:

pub struct BufTcpStream<'a> {
    input : BufReader<&'a TcpStream>,//   First to be dropped   |
    output: BufWriter<&'a TcpStream>,//   Second to be dropped  |
    _socket: Pin<Box<TcpStream>>,//       Last to be dropped    V
}




Actually, the 'a lifetime in struct BufTcpStream<'a> is useless. Lifetime of the unsafe references can be set to 'static.

Here is the fixed implementation:

use std::io::{BufReader, BufWriter};
use std::net::TcpStream;
use std::pin::Pin;

pub struct BufTcpStream {
    input : BufReader<&'static TcpStream>,
    output: BufWriter<&'static TcpStream>,
    _socket: Pin<Box<TcpStream>>,
}

impl BufTcpStream {
    pub fn new(socket: TcpStream) -> Self {
        let pin = Box::pin(socket);
        unsafe {
            Self{
                input : BufReader::new(std::mem::transmute(&*pin)),
                output: BufWriter::new(std::mem::transmute(&*pin)),
                _socket: pin,
            }
        }
    }
    pub fn reach<'b>(&'b mut self) -> (
        &mut BufReader<&'b TcpStream>,
        &mut BufWriter<&'b TcpStream>
    ) {
        unsafe {
            std::mem::transmute((&mut self.input, &mut self.output))
        }
    }
}
Related