Adds support for UnixStream and UnixListener on Windows

[sullivan-sean]

Uses mios existing AFD approach for TCP/UDP to support AF_UNIX sockets on Windows. See https://github.com/tokio-rs/mio/issues/1609 for context

[Thomasdezeeuw]

@sullivan-sean also see https://github.com/rust-lang/socket2/pull/249.

[sullivan-sean]

I removed the dependency on tempfile and fixed the tests.

I think the current implementation for Windows actually confuses the notion of edge triggered events with oneshot notifications i.e. EPOLLET vs EPOLLONESHOT in epoll and so it was not correctly re-registering for events after successful io.

@sullivan-sean also see rust-lang/socket2#249.

Would you prefer I introduce a dependency on this library for the socket operations? Looks like it would be pretty easy to use this instead of much of the socket code I've added

[Thomasdezeeuw]

I removed the dependency on tempfile and fixed the tests.

👍

I think the current implementation for Windows actually confuses the notion of edge triggered events with oneshot notifications i.e. EPOLLET vs EPOLLONESHOT in epoll and so it was not correctly re-registering for events after successful io.

That's an issue we should fix separately from adding Unix sockets for Windows. Can you open an issue for this?

@sullivan-sean also see rust-lang/socket2#249.

Would you prefer I introduce a dependency on this library for the socket operations? Looks like it would be pretty easy to use this instead of much of the socket code I've added

No. We decided not to add any dependencies and focus on a mostly std based API, i.e. all the net types in there but non-blocking.

[sullivan-sean]

That's an issue we should fix separately from adding Unix sockets for Windows. Can you open an issue for this?

Issue created here https://github.com/tokio-rs/mio/issues/1612 - I can make a separate PR from https://github.com/sullivan-sean/mio/commit/c725b035a06738ee9cd3e9b017cd28c8cac60e31 which contains just the fix for that, if this is indeed an issue

[sullivan-sean]

I've removed the changes in draining behavior, which requires more discussion in https://github.com/tokio-rs/mio/issues/1611 and fixed the tests instead by adding a would block assertion in compliance with the current drain behavior on windows documented here.

All tests are passing and a naive windows implementation in tokio (just copy pasting the unix implementation https://github.com/sullivan-sean/tokio) seems to pass all of the unix stream + listener tests there as well

[notgull]

I think the usual strategy is to just wait for libstd to implement it and not get too hasty with a patch we'll have to revert once libstd gains support.

Is there a tracking issue for implementing it into the standard library, by the way?

[sullivan-sean]

There is an open issue since 2018 https://github.com/rust-lang/rust/issues/56533, and progress seems to have stagnated - the mentioned min_target_api_version RFC is no longer being worked on. It also seems like support for named pipes was added to mio without std support? Though to be fair maybe that added less code than this...

I imagine that the std implementation, whenever it arrives, would be a pretty straightforward drop-in given that the internal UnixStream and UnixListener structs in this PR are built to mirror the std::os::unix::net equivalents almost exactly.

If you would prefer to not add this to mio yet it would be really nice to expose a way to patch an event source like this into mio/tokio as an end developer (i.e. given that PollEvented is hidden in tokio it is quite hard to create a compatible source if it is not supported by mio/tokio out of the box).

[Thomasdezeeuw]

There is an open issue since 2018 rust-lang/rust#56533, and progress seems to have stagnated - the mentioned min_target_api_version RFC is no longer being worked on.

I think that was superseded by https://github.com/rust-lang/rfcs/pull/2523.

It also seems like support for named pipes was added to mio without std support? Though to be fair maybe that added less code than this...

True, but it's only 1k lines in a single file: https://github.com/tokio-rs/mio/blob/master/src/sys/windows/named_pipe.rs. This pr is larger.

I imagine that the std implementation, whenever it arrives, would be a pretty straightforward drop-in given that the internal UnixStream and UnixListener structs in this PR are built to mirror the std::os::unix::net equivalents almost exactly.

Hopefully, that's why I'm even considering this, but it would push back our v1 or we need to add an unstable feature.

If you would prefer to not add this to mio yet it would be really nice to expose a way to patch an event source like this into mio/tokio as an end developer (i.e. given that PollEvented is hidden in tokio it is quite hard to create a compatible source if it is not supported by mio/tokio out of the box).

That's basically https://github.com/tokio-rs/mio/issues/880, but TL;DR that's not really possible.

[sullivan-sean]

I think that was superseded by rust-lang/rfcs#2523.

👍

True, but it's only 1k lines in a single file: https://github.com/tokio-rs/mio/blob/master/src/sys/windows/named_pipe.rs. This pr is larger.

Still not a single file, but after removing unused functions this PR is significantly smaller. There are also a non-trivial number of changes I had to make to pass lint checks that are unrelated to the actual content of this PR, e.g. all of the changes in src/net/tcp/stream.rs, src/sys/unix/pipe.rs, src/sys/unix/uds/mod.rs and a few other files.

I imagine that the std implementation, whenever it arrives, would be a pretty straightforward drop-in given that the internal UnixStream and UnixListener structs in this PR are built to mirror the std::os::unix::net equivalents almost exactly.

Hopefully, that's why I'm even considering this, but it would push back our v1 or we need to add an unstable feature.

If you would prefer to not add this to mio yet it would be really nice to expose a way to patch an event source like this into mio/tokio as an end developer (i.e. given that PollEvented is hidden in tokio it is quite hard to create a compatible source if it is not supported by mio/tokio out of the box).

That's basically #880, but TL;DR that's not really possible.

If it doesn't make sense to add this to mio now, pre-std support for UDS on Windows, what would you advise for users who would like to use mio/tokio with UDS in a cross-platform way? Maintain a personal fork? or maybe turn this into a more up-to-date https://github.com/Azure/mio-uds-windows?

[sullivan-sean]

@Thomasdezeeuw any updates on this? Is this still something I should be working on and if so, should I be exposing the Windows blocking types per https://github.com/tokio-rs/mio/pull/1610#discussion_r956163129?

[Thomasdezeeuw]

@Thomasdezeeuw any updates on this? Is this still something I should be working on and if so, should I be exposing the Windows blocking types per #1610 (comment)?

I just haven't had the time to review this properly. Perhaps I'll have some this weekend.

[sullivan-sean]

• Can remove all code changes not related to UDS. If that means the CI fails we'll fix that in another pr as this one is large enough as it is.

👍

• I assume UnixDatagrams are not supported?

Correct, the datagram socket type is not supported by Windows implementation of Unix sockets (https://devblogs.microsoft.com/commandline/af_unix-comes-to-windows/#unsupportedunavailable)

• What is path forward regarding:
  • Getting these types into the standard library?
  • Using SocketAddr from the standard library (see #1527).

Looks like there's some recent movement on https://github.com/rust-lang/rust/issues/56533. I'm not familiar with how/if we could go about adding this to the standard library, and if so what the route would be. But I am happy to push for this with guidance on the approach. I imagine a SocketAddr type for Windows could also be added to std when the other uds types are.

• I'm too well versed in Windows' inheritance, do we need any no inheritances after calls to accept?

Seems like sockets returned from accept have the same properties as the listener socket they are called on - so if the listener has WSA_FLAG_NO_HANDLE_INHERIT set this is handled automatically. I will remove the explicit inheritance calls after accept. Then if somebody creates a listener with from_raw_socket, the streams returned from calling accept on that listener will have the same flag as the original socket, which I imagine is desired behavior.

• We'll probably want to wrap SocketAddr in a type in mio::net to keep the docs and methods in sync. It already has different methods and trait implementation in this pr.

I'll work on adding this

I didn't have time to review the tests, but ideally very little to nothing changes with them as that means that existing code doesn't have to change to support Windows UDS.

Thanks for the very thorough review of everything else, I know it's a big PR and appreciate your patience! I've minimized test changes as much as possible while accounting for the lack of blocking socket types in std for Windows.

[Thomasdezeeuw]

Looks like there's some recent movement on rust-lang/rust#56533. I'm not familiar with how/if we could go about adding this to the standard library, and if so what the route would be. But I am happy to push for this with guidance on the approach. I imagine a SocketAddr type for Windows could also be added to std when the other uds types are.

You can start with a small RFC or maybe ask it on the Rust Zulip chat. Then just implement the types in std::os::windows and later merge them with Unix, maybe under std::net. But the libs team can help with that.

• I'm too well versed in Windows' inheritance, do we need any no inheritances after calls to accept?

Seems like sockets returned from accept have the same properties as the listener socket they are called on - so if the listener has WSA_FLAG_NO_HANDLE_INHERIT set this is handled automatically. I will remove the explicit inheritance calls after accept. Then if somebody creates a listener with from_raw_socket, the streams returned from calling accept on that listener will have the same flag as the original socket, which I imagine is desired behavior.

👍 can you add a comment saying as much.

I didn't have time to review the tests, but ideally very little to nothing changes with them as that means that existing code doesn't have to change to support Windows UDS.

Thanks for the very thorough review of everything else, I know it's a big PR and appreciate your patience! I've minimized test changes as much as possible while accounting for the lack of blocking socket types in std for Windows.

👍

[Noah-Kennedy]

Where are we with this?

[sullivan-sean]

I've been away from my Windows machine and so haven't had a chance to fix the failing docs test. I will add the comment about WSA_FLAG_NO_HANDLE_INHERIT and try to fix the doc test tomorrow.

I posted in the rust-lang Zulip about adding Window UDS to std and the libs-api team seemed at least open to the idea on the condition that the failure mode on older platforms without UDS support is that "call[ing] the relevant functions...give[s] an error" which is the current behavior here for WinSock calls in general.

[lumattr]

How is this one doing? Are we waiting for the rust changes of does this just need a re-review?

[Thomasdezeeuw]

How is this one doing? Are we waiting for the rust changes of does this just need a re-review?

I think this is pretty close, we need to resolve some small things and need to make sure that the test behave the same on Unix and Windows (which is harder then it sounds).

[KolbyML]

Hi @sullivan-sean I was wondering if you are going to finish this? If something came up and your are too busy too I would be happy to take it off your hands :)

[sullivan-sean]

Closed in favor of https://github.com/tokio-rs/mio/pull/1667. Thanks @KolbyML for cleaning this up :)

[simonsan]

@sullivan-sean @KolbyML Both #1610 and #1667 are closed now. Do you think you can work together on this?