Add IoSource

[Thomasdezeeuw]

IoSource is a wrapper around a raw file descriptor or socket that implements event::Source.

This will replace SourceFd on Unix (not removed in this pr). I'll send some more pr that replace the usage of SourceFd on Unix and use them in the net types on Windows.

Updates #880
Updates #1187

/cc @dtacalau

[Thomasdezeeuw]

#1196 (the last commit) changes TcpStream to use this type, see it for example usage.

[carllerche]

Thanks for putting this together. I will need to spend more time going through it.

A quick question: if this is public, in what scenarios would a user use this? In my understanding, the AFD strategy only works for types that mio already supports out of the box.

[Thomasdezeeuw]

A quick question: if this is public, in what scenarios would a user use this? In my understanding, the AFD strategy only works for types that mio already supports out of the box.

Correct, but isn't the same true for SourceFd? It depends on the selector what file descriptors are supported.

[carllerche]

Right... such a refactor is good for internals, but I hesitate to make the type public due to limitations. I think SourceFd is good for Unix and we can document that it is passed through to the selector so see OS docs for limitations. On windows we would need a custom IOCP handler.

Anyway, I will try to make time to review in depth ASAP

[Thomasdezeeuw]

Right... such a refactor is good for internals, but I hesitate to make the type public due to limitations. I think SourceFd is good for Unix and we can document that it is passed through to the selector so see OS docs for limitations. On windows we would need a custom IOCP handler.

Fair. After the first round of reviews I'll make them private and we can see from there if they can be made public.

[dtacalau]

With my current understanding of the code I did not see the point of *nix SourceFd, seems like an extra level which can be dropped w/o losing something.

[dtacalau]

@Thomasdezeeuw @carllerche Imo we should not do any refactoring, major or minor, of the Windows implementation internals until custom IOCP handlers are implemented (#1047 and #526). Couple of reasons why I think that: current implementation is stable, no known bugs, it has tests supporting it, provides a good base for implementing a new Internal Selector and a new HandlerState. We would be in a better position to refactor having the custom iocp up and running along the Afd stuff. Any refactoring done now could be duplicate work or just code that won't be needed after the custom IOCP handlers. That's why I think this PR, and the now closed PRs #1182 and #1183, would make much sense afterwards.

Let me know what you think.

[Thomasdezeeuw]

With my current understanding of the code I did not see the point of *nix SourceFd, seems like an extra level which can be dropped w/o losing something.

SourceFd is the only way to use Mio with types defined outside of Mio itself. For example I use for mio-pipe and mio-signals.

@Thomasdezeeuw @carllerche Imo we should not do any refactoring, major or minor, of the Windows implementation internals until custom IOCP handlers are implemented (#1047 and #526). Couple of reasons why I think that: current implementation is stable, no known bugs, it has tests supporting it, provides a good base for implementing a new Internal Selector and a new HandlerState. We would be in a better position to refactor having the custom iocp up and running along the Afd stuff. Any refactoring done now could be duplicate work or just code that won't be needed after the custom IOCP handlers. That's why I think this PR, and the now closed PRs #1182 and #1183, would make much sense afterwards.

Let me know what you think.

I strongly disagree with "current implementation is stable, no known bugs", you're one of the people fixing plenty of bugs in the implementation with changes as recent as today. So I wouldn't call it stable or bug free. Furthermore if we get "scared" (for lack of a better word) to change the code it simply means we don't have enough test coverage in my opinion.

If all net types (i.e. UdpSocket, TcpStream and TcpListener) would use IoSource the number of lines (and possible places of bugs) and code duplication in the Windows implementation would be greatly reduced as shown in bb1e93f.

[dtacalau]

SourceFd is the only way to use Mio with types defined outside of Mio itself.

@Thomasdezeeuw thanks for explaining usage of SourceFd, I've reviewed the docs, makes more sense now.

As for my other opinion expressed above, "Scared" might not be the proper word, I'd say "cautious" if you want a word to describe it :).

[kleimkuhler]

So this looks really good to me so far. I don't really have any blocker feedback for the logic. It's a little unfortunate Window's selector has so much going on in it, but I that the separation of what IoSourceState is responsible for in {re,de}registering is similar to the net resources.

I'm interested to see how this cleans up other parts of the Windows implementation which I'm assuming a few more changes will be taking place in the PR, but this is a 👍 start to me.

[Thomasdezeeuw]

Postponing until next week, see #1196 (comment).

[Thomasdezeeuw]

Closing in favour of #1236.