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

Amend RFC 517: Add material on std::net #807

Merged
merged 3 commits into from
Feb 18, 2015
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
160 changes: 158 additions & 2 deletions text/0517-io-os-reform.md
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,10 @@ follow-up PRs against this RFC.
* [stdin, stdout, stderr]
* [std::env]
* [std::fs] (stub)
* [std::net] (stub)
* [std::net]
* [TCP]
* [UDP]
* [Addresses]
* [std::process] (stub)
* [std::os]
* [Odds and ends]
Expand Down Expand Up @@ -1234,7 +1237,160 @@ This brings the constants into line with our naming conventions elsewhere.
### `std::net`
[std::net]: #stdnet

> To be added in a follow-up PR.
The contents of `std::io::net` submodules `tcp`, `udp`, `ip` and
`addrinfo` will be retained but moved into a single `std::net` module;
the other modules are being moved or removed and are described
elsewhere.

#### TCP
[TCP]: #tcp

The current `TcpStream` struct will be pared back from where it is today to the
following interface:

```rust
// TcpStream, which contains both a reader and a writer

impl TcpStream {
fn connect<A: ToSocketAddrs>(addr: &A) -> io::Result<TcpStream>;
fn peer_addr(&self) -> io::Result<SocketAddr>;
fn socket_addr(&self) -> io::Result<SocketAddr>;
fn shutdown(&self, how: Shutdown) -> io::Result<()>;
fn duplicate(&self) -> io::Result<TcpStream>;
}

impl Read for TcpStream { ... }
impl Write for TcpStream { ... }
impl<'a> Read for &'a TcpStream { ... }
impl<'a> Write for &'a TcpStream { ... }
#[cfg(unix)] impl AsRawFd for TcpStream { ... }
#[cfg(windows)] impl AsRawSocket for TcpStream { ... }
```

* `clone` has been replaced with a `duplicate` function. The implementation of
`duplicate` will map to using `dup` on Unix platforms and
`WSADuplicateSocket` on Windows platforms. The `TcpStream` itself will no
longer be reference counted itself under the hood.
* `close_{read,write}` are both removed in favor of binding the `shutdown`
function directly on sockets. This will map to the `shutdown` function on both
Unix and Windows.
* `set_timeout` has been removed for now (as well as other timeout-related
functions). It is likely that this may come back soon as a binding to
`setsockopt` to the `SO_RCVTIMEO` and `SO_SNDTIMEO` options. This RFC does not
currently proposed adding them just yet, however.
* Implementations of `Read` and `Write` are provided for `&TcpStream`. These
implementations are not necessarily ergonomic to call (requires taking an
explicit reference), but they express the ability to concurrently read and
write from a `TcpStream`

Various other options such as `nodelay` and `keepalive` will be left
`#[unstable]` for now. The `TcpStream` structure will also adhere to both `Send`
and `Sync`.

The `TcpAcceptor` struct will be removed and all functionality will be folded
into the `TcpListener` structure. Specifically, this will be the resulting API:

```rust
impl TcpListener {
fn bind<A: ToSocketAddrs>(addr: &A) -> io::Result<TcpListener>;
fn socket_addr(&self) -> io::Result<SocketAddr>;
fn duplicate(&self) -> io::Result<TcpListener>;
fn accept(&self) -> io::Result<(TcpStream, SocketAddr)>;
fn incoming(&self) -> Incoming;
}

impl<'a> Iterator for Incoming<'a> {
type Item = io::Result<TcpStream>;
...
}
#[cfg(unix)] impl AsRawFd for TcpListener { ... }
#[cfg(windows)] impl AsRawSocket for TcpListener { ... }
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It would be great to be able to create TcpStream, TcpListener, and UdpSocket from an AsRawFd or AsRawSocket as well (e.g. file descriptor passing or cross I/O library interface).

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree that this would be quite useful! I think we may want to develop our story around these raw representations a little more before exploring this area, however. The AsRawFoo traits are pretty new and I'd want to make sure that they're sufficient before we start adding conversions in the other direction.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is, however, other interest in this, so we may be able to at least sketch out something #[unstable] for now.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

```

Some major changes from today's API include:

* The static distinction between `TcpAcceptor` and `TcpListener` has been
removed (more on this in the [socket][Sockets] section).
* The `clone` functionality has been removed in favor of `duplicate` (same
caveats as `TcpStream`).
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What's the reasoning for not using clone here?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The Clone trait primarily indicates that the clone operation itself is infallible whereas the system call dup can fail (file descriptor exhaustion). I suspect that WSADuplicateSocket may also have some similar error conditions.

* The `close_accept` functionality is removed entirely. This is not currently
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So there's no proposed way to shutdown a server/listener?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have been personally unable to come up with a cross-platform method of doing so which doesn't require TcpListener to be anything more than a file descriptor, but I would certainly love to have this implemented! The impetus of paring the definition of TcpListener back down to just a file descriptor is what motivates this removal for now.

This RFC is largely targeted at taking a conservative step for the std::net primitives, but it should definitely be possible to add a select abstraction (if necessary) in the future. Additionally, it should be possible to develop out-of-tree solutions for now via the AsRawFd trait implementation.

implemented via `shutdown` (not supported well across platforms) and is
instead implemented via `select`. This functionality can return at a later
date with a more robust interface.
* The `set_timeout` functionality has also been removed in favor of returning at
a later date in a more robust fashion with `select`.
* The `accept` function no longer takes `&mut self` and returns `SocketAddr`.
The change in mutability is done to express that multiple `accept` calls can
happen concurrently.
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

+1 for the concurrent accept. Seems pretty standard, and would work fine for my toy use case at least. EDIT: as long as there's no hidden select() giving you a gun-at-foot thundering herd problem...

* For convenience the iterator does not yield the `SocketAddr` from `accept`.

The `TcpListener` type will also adhere to `Send` and `Sync`.

#### UDP
[UDP]: #udp

The UDP infrastructure will receive a similar face-lift as the TCP
infrastructure will:

```rust
impl UdpSocket {
fn bind<A: ToSocketAddrs>(addr: &A) -> io::Result<UdpSocket>;
fn recv_from(&self, buf: &mut [u8]) -> io::Result<(usize, SocketAddr)>;
fn send_to<A: ToSocketAddrs>(&self, buf: &[u8], addr: &A) -> io::Result<usize>;
fn socket_addr(&self) -> io::Result<SocketAddr>;
fn duplicate(&self) -> io::Result<UdpSocket>;
}

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fwiw, there are useful reasons for calling connect() on a UDP socket:

a) In many protocols you are exchanging a stream of UDP packets with a single peer, and connect() can simplify your internal API.

b) In some (non-NAT-friendly) protocols (eg: RTP), you need to find a local IP address to tell the other end about. A reasonably common/portable/clean way to do this is to open a UDP socket, connect() to the peer IP to force the kernel to bind the local end, and then use getsockname() to fetch the IP address the kernel decided to use.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree! I believe that this should be done through a future Socket API though instead of having a large number of constructors on UdpSocket.


#[cfg(unix)] impl AsRawFd for UdpSocket { ... }
#[cfg(windows)] impl AsRawSocket for UdpSocket { ... }
```

Some important points of note are:

* The `send` and `recv` function take `&self` instead of `&mut self` to indicate

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

send_to and recv_from?

that they may be called safely in concurrent contexts.
* All configuration options such as `multicast` and `ttl` are left as
`#[unstable]` for now.
* All timeout support is removed. This may come back in the form of `setsockopt`
(as with TCP streams) or with a more general implementation of `select`.
* `clone` functionality has been replaced with `duplicate`.

The `UdpSocket` type will adhere to both `Send` and `Sync`.

#### Sockets
[Sockets]: #sockets

The current constructors for `TcpStream`, `TcpListener`, and `UdpSocket` are
largely "convenience constructors" as they do not expose the underlying details
that a socket can be configured before it is bound, connected, or listened on.
One of the more frequent configuration options is `SO_REUSEADDR` which is set by
default for `TcpListener` currently.

This RFC leaves it as an open question how best to implement this
pre-configuration. The constructors today will likely remain no matter what as
convenience constructors and a new structure would implement consuming methods
to transform itself to each of the various `TcpStream`, `TcpListener`, and
`UdpSocket`.

This RFC does, however, recommend not adding multiple constructors to the
various types to set various configuration options. This pattern is best
expressed via a flexible socket type to be added at a future date.

#### Addresses
[Addresses]: #addresses

For the current `addrinfo` module:

* The `get_host_addresses` should be renamed to `lookup_host`.
* All other contents should be removed.

For the current `ip` module:

* The `ToSocketAddr` trait should become `ToSocketAddrs`
* The default `to_socket_addr_all` method should be removed.

The actual address structures could use some scrutiny, but any
revisions there are left as an unresolved question.

### `std::process`
[std::process]: #stdprocess
Expand Down