RustConnection::poll_for_event never reads from the socket

[psychon]

Currently, there is no way to get events in a non-blocking way. Only wait_for_event() actually reads from the socket.

I guess poll_for_event() has to somehow check if there is currently a reader and if not, try to read. No idea how this could fit into the Read trait.

[eduardosm]

I'm afraid this would require non-blocking reading... How important is to have RustConnection generic over BufReadFD and BufWriteFD instead of just using Stream?

[psychon]

I'm afraid this would require non-blocking reading...

Which is still compatible with the Read trait (and thus also with ReadFD), I think. We'd just have to require a non-blocking behaviour of the implementation.

How important is to have RustConnection generic over BufReadFD and BufWriteFD instead of just using Stream?

I am not quite sure what you are asking here. For the sake of argument, let's say that all generic arguments should be removed. What exactly would you propose to do? (I guess one can still add back generics to any solution we come up with)

Right now, the only solution that I see would be to require non-blocking reading. Then, we could add another generic something to RustConnection that abstract over poll. This "something" could then also be implemented on Windows (is there something similarly nice to the nix crate for windows? A quick Google search led me to https://docs.rs/ws2_32-sys/0.2.1/i686-pc-windows-gnu/ws2_32/fn.WSAPoll.html which is unsafe...)

Edit: https://docs.rs/winapi/0.3.8/winapi/um/winsock2/fn.WSAPoll.html seems to be the way to go, but is unsafe.https://crates.io/crates/winapi-util does not seem to provide the necessary API.

[eduardosm]

I am not quite sure what you are asking here. For the sake of argument, let's say that all generic arguments should be removed. What exactly would you propose to do? (I guess one can still add back generics to any solution we come up with)

I just thought it would be easier implement without generics (e.g., using set_nonblocking). Of course, we could create a trait that provides that method.

This "something" could then also be implemented on Windows (is there something similarly nice to the nix crate for windows? A quick Google search led me to https://docs.rs/ws2_32-sys/0.2.1/i686-pc-windows-gnu/ws2_32/fn.WSAPoll.html which is unsafe...)

That's why I had proposed to use mio, which solves the portability issue (but would likely require to remove the generic parameters).

[dalcde]

You should look at how smol does this (https://github.com/stjepang/smol). iirc on Unix systems, it suffices for the Read object to implement AsRawFd, and you can set the nonblocking mode on the fd; on Windows systems it is more complicated.

[eduardosm]

Thinking better about it, no matter what library we use, RustConnection can be kept generic with a trait like this one...

trait NonBlockingStream: ReadFD + WriteFD {
    fn poll(&mut self, block: bool) -> io::Result<PollEvents>;
}

Then, we could add another generic something to RustConnection that abstract over poll.

... which I believe that is what you meant here. But in this case it joins the reader and writer into a single object.

[psychon]

Thanks for the ideas.

iirc on Unix systems, it suffices
for the Read object to implement AsRawFd, and you can set the
nonblocking mode on the fd; on Windows systems it is more complicated.

Seems like it. The relevant magic is here, I would say:
https://github.com/stjepang/smol/blob/2071f2a481e64459a99173b384afb33f12c18068/src/reactor.rs#L103-L114

I was thinking about something like this (very rough sketch)

trait Poll<T, R, W> {
    // This should be an abstraction over AsRawFd and AsRawSocket, with T being either RawFd or Socket
    fn extract_token(&self, either_R_or_W) -> T;
    // This would be the actual poll() call
    fn poll(&self, read: &[T], write: &[T]) -> dunno;
}

RustConnection would then also get an implementation of the above trait. (I guess T needs to be Send + Sync and perhaps even 'static? Perhaps it also makes more sense to split this up into two traits and what not, but this is the general idea that I have floating around so far).

Perhaps it makes more sense to just reinvent the whole PollFd thing as a trait instead and hope for the compiler to optimise the extra level of abstraction away: https://docs.rs/nix/0.17.0/nix/poll/fn.poll.html

Perhaps more realistic:

/// An abstraction over `AsRawFd` and `AsRawSocket`
trait AsToken {
    type Token;
    fn as_token(&self) -> Token;
}
/// An abstraction over `poll()` and `WSAPoll()`.
trait Poll {
    type Token;
    fn poll(&self, our-own-copy-of-the-poll-API) -> poll-return;
}

RustConnection would then end up very complicated:

struct RustConnection<R, W, P, T>
where
    R: ReadFd,
    R: AsToken<Token=T>,
    W: WriteFd,
    R: AsToken<Token=T>,
    P: Poll<Token=T>,
    T: Send + Sync + 'static // or whatever is needed for it

Oh and the API docs would then just mention that R and W have to behave non-blocking-ly.

For Windows, one could then use WSAPoll for basically the same result.

[psychon]

After some more thinking... I am inclined to agree that all the generic stuff could be removed. It would actually simplify lots of things. I am a bit sad about it (since it e.g. means that the reading test needs to be removed), but perhaps it is better this way.

What I just noticed: #222 cannot really be sanely fixed without integrating the reading and writing parts a lot more. At that point, a lot of the genericity would be lost.

Perhaps it is best to remove the generic arguments for now, fix this issue and #222 and only afterwards start thinking about how a generic version of this could look like.

[psychon]

I hacked together a quick demo on how things could work like: https://gist.github.com/psychon/8c12a9b201b56c72c44a2a832ae1dce4

This uses poll via the nix crate. It does not have the problem of #222: It always poll()s for reading whenever it wants to write.

After having written this... well. Actionable items would be: Stop using APIs like write_all and write_vectored_all. They just cannot work in non-blocking mode.

We want multi-thread support, so writing should be separate from reading. Currently, we achieve this with TcpSocket::try_clone(). This gives us one handle for reading and one for writing.

At the same time, #222 should be fixed, so writing also needs to be able to read. So... I have no idea how integrate this with Rust. Who borrows what when? I imagine the API for writing gets the read half as an argument... or something.

I guess I'll experiment with a unix-only API that uses RawFd instead of something fancy like TcpStream.