Implement Clone for TcpStream

[yoshuawuyts]

As per #365 (comment) we should implement Clone for TcpStream. The inner fd is already wrapped in an Arc, so cloning it should be straightforward. Thanks!

[k-nasa]

After all need a source clone, right?
I think you need to call mio :: tcp_stream :: try_clone.
If so, this should be try_clone, not clone.

pub struct Watcher<T: Evented> {
    /// Data associated with the I/O handle.
    entry: Arc<Entry>,

    /// The I/O event source.
    source: Option<T>,
}

[yoshuawuyts]

@k-nasa I believe @stjepang argued this should be clone instead because we already have an Arc<TcpStream> internally; so there's no need to do an extra syscall to obtain an extra handle to it.

[yoshuawuyts]

Seems like this was accidentally closed (:

[atbentley]

@yoshuawuyts, it looks like @stjepang is saying that async_std::net::TcpStream currently doesn't use an Arc internally, but it could?

If that's the case, I started having a look at what it would take to do that. I changed the TcpStream struct to be:

#[derive(Debug)]
pub struct TcpStream {
    pub(super) watcher: Watcher<Arc<mio::net::TcpStream>>,
}

And updated all the references to the inner value of TcpStream to respect the Arc. I ran into an issue with the IntoRawFd trait though. What would this trait look like if we use an Arc? My understanding of rust is still pretty surface level but it seems like into_raw_fd will move the value of the fd, which will be impossible here because Arc shares the reference to potentially many copies.

    impl IntoRawFd for TcpStream {
        fn into_raw_fd(self) -> RawFd {
            self.watcher.into_inner().as_ref().into_raw_fd()
        }
    }

So I just commented it out and proceeded with the Clone trait. Which I tested using some toy examples and that seemed to work.

impl Clone for TcpStream {
    fn clone(&self) -> Self {
        TcpStream {
            watcher: Watcher::new(self.watcher.get_ref().clone())
        }
    }
}

Is this a good approach though? I'm not exactly sure on the internals of async_std, but it looks like it I call Watcher::to_inner and my source is cloned many times then I may end up killing the clones? Also TcpStream.watcher.entry makes use of an Arc too, but I'm not cloning it, I would need to somehow expose that if that's a useful thing to clone.

On the other hand, is try_clone just a matter of?

    pub fn try_clone(&self) -> io::Result<TcpStream> {
        Ok(TcpStream {
            watcher: Watcher::new(self.watcher.get_ref().try_clone()?)
        })
    }

I'm happy to open a pull request if any of these seem like reasonable ideas.

[gsquire]

It just dawned on me. Could you presumably clone a std::net::TcpStream, and then convert it into an asynchronous one via the From implementation?

[yoshuawuyts]

@atbentley it seems try_clone is indeed very simple to implement; I think adding that would be great and would unblock people. We can later always investigate whether it's possible to implement Clone.

@gsquire std::net::TcpStream doesn't implement Clone, so we'd have to go through try_clone + unwrap instead, at which point exposing try_clone directly seems like the better option for now.

[rylev]

I have a use case where having Clone on a TcpStream would be very helpful. Happy to share details if needed.

[xpepermint]

Is this issue still unresolved? I'm asking because it already works for me:

...
spawn(read_stream(stream.clone()));
spawn(write_stream(stream)).await?;

[ghost]

You're right, this issue is resolved :)

[xpepermint]

Good! @yoshuawuyts please close this issue.