Consider decorating net types with non-blocking marker about mio

[fhartwig]

For the wrapper type name, I like NonBlocking best. I think it's much clearer than the other two options.
Why should the wrapper type have a type parameter? That is, why not NonBlockingTcpStream instead
of NonBlocking<TcpStream>. Are there other non-blocking things that you would want to wrap? Maybe I'm misunderstanding the design.

[carllerche]

@fhartwig Every type that has a blocking variant could have a non-blocking variant. TcpStream, TcpListener, TcpSocket, UdpSocket, UnixListener, UnixStream, and probably more...

[elinorbgr]

I really like the NonBlocking<_> approach, even if it's a little long, it's very explicit about what it's doing.

About the similarity with NonBlock<_>, maybe it's this second that could use a renaming ? Yet I don't really know what to put instead...

[fhartwig]

@carllerche That makes sense. How would you expose the underlying socket's methods?

[carllerche]

@fhartwig I would implement them on NonBlocking

impl NonBlocking<TcpSocket> {
  fn connect(self) -> NonBlocking<TcpStream> {
    // impl
  }
}

[fhartwig]

That would mean having to write wrapper methods for all methods of the underlying object that you want to expose, right?
One aspect to consider is whether the wrapper type would actually be sufficient to guarantee that the underlying socket is in non-blocking mode. Could the user e.g. call try_clone on a TcpStream, then convert one of the streams to a Nb<TcpStream> and then change back to blocking mode through the non-wrapped TcpStream?

[carllerche]

@fhartwig nonblock is a property of the file descriptor, not the underlying file (socket). So, in the case that you are describing would be fine. TcpStream would be blocking and Nb<TcpStream> would be non-blocking even though both values represent the same open socket. They would have different FDs.

[carllerche]

I am currently leaning towards using NonBlock as the wrapper type. So, having NonBlock<TcpStream> as the type that is used to track a non-blocking connected TCP socket. I propose to get rid of the current NonBlock type in favor of Option.

So, for example:

use std::io;

impl NonBlock<TcpStream> {
    fn read<B: MutBuf>(&self, dst: &mut B) -> io::Result<Option<usize>> {
        // ...
    }
}

Thoughts?

[reem]

I support this proposal.

[elinorbgr]

Same here, having NonBlock<_> being simply "Option<_> with an other name" does not feel very necessary when the information about non-blocking-ness is already handled by the type of the handler to the socket.

[fhartwig]

@carllerche On my machine, man 2 dup contains the sentence

Thus if fildes2 and fildes are duplicate references to an open file, [..] append mode, non-blocking I/O and asynchronous I/O options are shared between the references.

I'm probably misunderstanding something, but to me that sounds like having multiple file descriptors for a socket would allow violating Nb<_>'s invariants.

[carllerche]

@fhartwig Looks like you are correct and I misread the docs.

So, TcpStream doesn't have any guarantees. NonBlock<TcpStream> should be non-blocking. There are a couple options considering that setting O_NONBLOCK is not actually exposed.

a) Don't allow conversions between Foo and NonBlock<Foo>. Then, if somebody removes O_NONBLOCK by getting at the FD, such is life...

b) Allow conversions to NonBlock<Foo> and if the user manually removes O_NONBLOCK on another FD, such is life.

[reem]

It appears to me that the issue is that the AsRawFd trait (or any other conversion methods) is actually a huge leak in any further abstraction, since as long as you can get a raw file descriptor of the io object in question, you can do anything.

As a result, I don't think there's really anything we can do to prevent abstraction leaks or to prevent invariants about settings and flags from being broken. This makes me question the purpose of the NonBlock wrapper in the first place, since it actually can't enforce the invariant that O_NONBLOCK is actually set.

That said, it seems bad for mio to silently alter the behavior of existing types, since that means it is easy to misuse mio with libraries built for blocking io. If we do decide to continue with the NonBlock path, I think that option b from @carllerche's post above is probably the way to go. We should allow (checked) conversions to and from NonBlock and just document that removing O_NONBLOCK by getting another fd to the same socket is bad behavior.

EDIT: Anything more involved seems like a bad fit for mio, considering the low-cost-ness of the library.

[fhartwig]

Yeah, I guess in the presence of AsRawFd, the whole cloning issue was a bit of a red herring, sorry.
While it would have been nice to guarantee that Nb<Foo> is non-blocking, I think it's worth having the wrapper type anyway, even if it's just for the reasons mentioned by @reem

[carllerche]

This has been done