TcpStream & TcpListener rely on internals

[damonbarry]

I'm working on UDS support for Windows. I hope to start with my own mio-uds-windows crate and maybe get it merged into mio-uds at some point. I'm new to all this so I'm probably missing something obvious, but I've had a really hard time finding a good starting point for my Windows UnixStream and UnixListener.

The obvious choice was to start with mio's TcpStream and TcpListener. But I can't build them outside of the mio codebase. They make use of SelectorId, which isn't public. I can make a copy of the SelectorId code, but it accesses the private selector field on Poll. Also, they wrap the public mio::windows::Binding with ReadyBinding. When I copy that type, it won't build because it accesses the private selector field to get/put buffers down in BufferPool.

I could start from mio-named-pipe, but it only implements the stream not the listener, it doesn't implement read_bufs/write_bufs (even if it did, it wouldn't be able to use the internal BufferPool), and it preallocates memory and does an extra copy for reads (8K).

Anyway, I'm wondering if it would be possible to extricate TcpStream and TcpListener in such a way that they make a good example for other streams and listeners, but without breaking things?

[carllerche]

Sorry for the delay. It looks like you found a work around? I'm not entirely sure I follow the original question.

In general, the windows impl strategy is most likely going to significantly change in the future. I expect we will use a strategy similar to http://github.com/piscisaureus/wepoll.

[damonbarry]

@carllerche Yes I found a workaround, although it's not something we want to stick with long-term.

@arsing describes the problem more succinctly in this comment on deprecrated/mio-uds#7.

[arsing]

@carllerche

The problem we're trying to solve is that we want mio-uds and downstream crates to support Unix Domain Sockets on Windows. Currently we have a hacky way of doing this that involves duplicating a lot of code from libstd, mio and mio-uds into https://github.com/Azure/mio-uds-windows and using [replace] to swap out mio-uds for that repo. But we want to work out a way to upstream this into mio-uds in the long term.

As Damon mentioned in the OP of this issue and I wrote in deprecrated/mio-uds#7 (comment), the mio::windows::TcpListener and TcpStream encapsulate working with IOCP and buffer pools but this functionality is not separately exposed by mio::windows. So it's not possible to write a listener and stream in mio-uds that can also use the system selector in the same way without duplicating all that code.

I wanted to see if it was possible to abstract out the internals of TcpListener and TcpStream that are not TCP-specific into a separate public type, that can then be parameterized to work with mio's TCP types and mio-uds's UDS types.

That change is here. It creates a bunch of new traits that can be implemented by any SOCKET-based listener and stream type outside mio, and two new SocketListener<TListener: Listener> and SocketStream<TStream: Stream> types that contain the bulk of generic functions that TcpListener and TcpStream used to have but are not specific to TCP sockets.

With this, mio-uds can have

struct UnixListenerImp;

impl mio::windows::Listener for UnixListenerImp { ... }

struct UnixListener {
    inner: SocketListener<UnixListenerImp>,
}

struct UnixStreamImp;

impl mio::windows::Stream for UnixStreamImp { ... }

struct UnixStream {
    inner: SocketStream<UnixStreamImp>,
}

What do you think of this approach? Would it be something that can be merged into mio 0.6 ? Is there a better way of doing this?

[carllerche]

@arsing Yes, right now a lot of mio's internals have to be copied out, which is not great.

I would like to re-implement Mio's windows support using the wepoll strategy. Once this happens, socket types will be able to use the regular non-OVERLAPPED Winsock API. This will make things much easier.

[arsing]

Right. As I said in our email conversation, if the wepoll implementation results in an EventedSocket just like the existing EventedFd, then it should make the mio-uds part trivial.

[yorickpeterse]

@carllerche I ran into the need for wepoll/epoll/kqueue myself, as part of my work on Inko. Since MIO has various issues with Windows, and the API isn't stable (and seems to be scheduled for lots of changes based on the various issues), I ended up writing my own wrappers around these libraries. For epoll and kqueue I used the nix crate, but for wepoll I had to write my own wrappers:

• https://crates.io/crates/wepoll-binding: safe bindings to wepoll
• https://crates.io/crates/wepoll-sys: raw bindings to wepoll

The wepoll-sys crate bundles the wepoll code and compiles it upon installation, as I don't have the time and resources to port the code to Rust. The wepoll-binding crate provides a safe interface around wepoll-sys. The safe bindings are fairly lightweight, and the API is based on (and should feel similar to) MIO. Using these crates might be useful for MIO, and I'm happy to review any patches MIO might need to make things work.

I haven't gotten to extensively testing both though (only minor testing to get CI going), so there might be some bugs.

p.s. should MIO be interested in using these crates, it might be worth renaming wepoll-binding to just wepoll, but that name is currently reserved.

[Thomasdezeeuw]

If I'm reading this correctly basically this issue asking for a SourceFd (previously EventedFd) equivalent for Windows. This is on the list of item to after the rewrite of the Windows implementation, see #1041. @damonbarry, @arsing could you share your ideas on how that should look for your use cases?

[arsing]

If I'm reading this correctly basically this issue asking for a SourceFd (previously EventedFd) equivalent for Windows.

That's right; see #880 (comment)

It would presumably be SourceSocket and wrap a SOCKET. I would guess wepoll makes the event::Source impl identical for both.

Then the UDS side can forward to SourceSocket just like the existing mio-uds forwards to EventedFd today.

[Thomasdezeeuw]

I'm not a Windows guy, but would also need a wrapper around a HANDLE then (for files if I'm correct)? Although I'm not sure if there is any non-blocking filesystem stuff on Windows.

[arsing]

The wepoll approach won't work for file HANDLEs. It only works for things managed by the AFD driver.

[Thomasdezeeuw]

Now that #1064 is merged I'm working on this, I intend to provide a SourceFd like abstraction for at least socket (and maybe handles if that is possible).

[carllerche]

@Thomasdezeeuw what would this look like? I'm not sure how something like that would work for windows, except maybe: #1047

[Thomasdezeeuw]

@carllerche my current idea is to add a wrapper type that adds InternalState and change all net types to use the new type, just like on Unix. The only problem is re-registering on would block errors. Currently this is on done in try_io, I was think about adding method do_io that would replicate the macro.

[carllerche]

oh, you mean an internal refactor?

[Thomasdezeeuw]

Yes, but also make the wrapper type public.

[Thomasdezeeuw]

I've created #1183, I don't really like the API but it follows SourceFd. We should discuss in #1183, but maybe we should consider making the fd/socket owned.

[carllerche]

I'm just not sure how we can make the wrapper type public. It is pretty specific. I would love to hear your planned strategy.

[Thomasdezeeuw]

@carllerche I already implementation something here: https://github.com/tokio-rs/mio/pull/1183/files#diff-2ace2cbf884d3de3ae825a07c3e5bffdR101. Usage in UdpSocket: https://github.com/tokio-rs/mio/pull/1183/files#diff-c9ef904d3020aa1431eaa718ee98c307R153-R154.

But like I said I don't really like the API as its rather fragile to use, but its modelled after SourceFd.

[carllerche]

Right, I see how you could model an abstraction that is used across TCP & UDP internally. I just don't see how it can become a public API.

[Thomasdezeeuw]

This isn't going into alpha1.

[Thomasdezeeuw]

We still don't have a good solution for this, but I want to release v0.7 so punting this to v1.

[Thomasdezeeuw]

This is not going to block a v1.0 release.

[Thomasdezeeuw]

We're keeping the current AFD based implementation, so there isn't anything to do here.