WIP: Implement FromRawFd for TcpStreamNew

[rushmorem]

Implements std::os::unix::io::FromRawFd for net::TcpStreamNew.
This should make it easier to utilise futures on existing connections.

This pull request builds on top of #84
so if both are desirable you can just merge this one and close
#84.

[alexcrichton]

Thanks for the PR! I'd be pretty wary about adding an impl like this which can so easily and silently fail. This was also in theory the purpose of the TcpStream::connect_stream method, construction from an arbitrary other TCP stream. Perhaps that method could be used instead to create a std::net::TcpStream and then convert that to a tokio_core::net::TcpStream?

[rushmorem]

Sometimes this is the only way to convert an existing TcpStream. TcpStream::connect_stream moves and takes ownership ownership of a TcpStream. This will not work if you can only borrow an instance of a TcpStream but can't Copy or Clone it. I actually ran into this with the r2d2 connection pool.

Having said that, this implementation will not work because of it's reliance on #84. Instead of implementing FromRawFd I think it's better to have an extra parameter that borrows a handle and use that instead. I have already implemented it like this here. What do you think about that implementation? Can we use that instead?

[alexcrichton]

Yeah passing an explicit handle sounds good to me, but I'm not sure I quite follow why connect_stream can't be used. If you can only borrow an instance of TcpStream, then you can't pass ownership of the file descriptor as well?

[rushmorem]

That's just the thing! std::os::unix::io::AsRawFd borrows an instance of an object but returns an owned file descriptor which is what makes this whole thing work.

[alexcrichton]

Hm ok so you claimed:

Sometimes this is the only way to convert an existing TcpStream. ... This will not work if you can only borrow an instance of a TcpStream but can't Copy or Clone it.

If all you have is a borrow, it's not safe to call this function. So if all you have is a borrow, you can never create a tokio_core::net::TcpStream. In that sense, I think we can set that use case aside as it's not possible to do any of these conversions.

In light of that the connect_stream method should be sufficient, right?

[rushmorem]

We can't create a tokio_core::net::TcpStream but we can create a tokio_core::net::TcpStreamNew instead which is what tokio_core::net::TcpStream::connect does anyway.

As for safety, yes this function is unsafe so it should be used as a last resort. For example the docs for FromRawFd say:-

This function is also unsafe as the primitives currently returned have the contract that they are the sole owner of the file descriptor they are wrapping. Usage of this function could accidentally allow violating this contract which can cause memory unsafety in code that relies on it being true.

The signature of the function (unsafe fn from_raw_fd(fd: RawFd) -> Self) clearly communicates this so I don't think anyone would be caught with their guard down.

[alexcrichton]

No the TcpStreamNew also takes ownership, right? If all you have is a reference you basically just can't call FromRawFd in any way shape or form?

[alexcrichton]

I believe this is largely subsumed by TcpStream::from_stream, so closing.