Add API to hook into Windows IOCP loop #1345

darkwisebear · 2020-08-11T05:57:52Z

An API similar to the one of mio 0.6 is now available so that custom objects can be added to the IOCP that is used to implement Poll. Relates to #1047.

This change isn't complete yet but in a draft state as I wanted to gather some feedback on the suggested API.

Since sockets are polled by using a single connection to afd.sys, there is no 1:1 relationship between a user-provided token and the completion key of the IOCP. The implementation therefore uses a slab to keep track of the registered handlers and the mapping of the user-provided token to the reported events.

Other notable things about the change:

The Overlapped type now uses a boxed trait object instead of a function pointer. While this is resource-heavier, it also enables writing more clean completion handlers as they can now directy carry state instead of having to do some pointer magic or accessing some sort of global state.
There is anew type "Readiness" to convert the completion events into polling events. There is also a similar (equally named) type inside the tests and maybe this type is misplaced inside the Windows module.
I still need to write more tests and add them to this PR before it can be accepted. Working on it.

An API similar to the one of mio 0.6 is now available so that custom objects can be added to the IOCP that is used to implement Poll.

This is a first test to play with the Windows IOCP handler API.

Thomasdezeeuw

I'm only looked at the surface level API, not at the implementation and added a few comment.

Overall I'm wondering if its possible to create something more closely resembling the SourceFd API but for RawHandles. So far we exposed very little of the internals of Mio, e.g. no epoll/kqueue specific types but this exposed quite a lot of IOCP I'm not sure about that.

Thomasdezeeuw · 2020-08-13T08:23:20Z

src/lib.rs

+/// * `Binding` - this type is intended to govern binding with mio's `Registry`
+///   type. Each I/O object should contain an instance of `Binding` that's
+///   interfaced with for the implementation of the `Source` trait. The
+///   `register`, `reregister`, and `deregister` methods for the `Source` trait


I see that Binding only has register_handle, how would I implement de/re-register?

Or this is that part simply not implemented yet?

Its not implemented yet because I'm unsure how that should/could be done as in Windows, there is no way of removing a handle from an IOCP except by closing that handle. So the only thing we should do here is to remove the handler from the list of IO handlers, which should be done by a Drop implementation of that type. reregistration would only support changing the token; I will add both functionalities.

Thomasdezeeuw · 2020-08-13T08:32:33Z

src/sys/windows/selector.rs

+        /// I/O operations that are registered with mio's event loop. When the I/O
+        /// operation associated with an `OVERLAPPED` pointer completes the event
+        /// loop will invoke the object provided as `callback`.
+        pub fn new<C: CompletionCallback+'static>(callback: C) -> Self {


Why do we need a trait with a single method? Can't we useC: FnMut(&OVERLAPPED_ENTRY) + static?

Also can't Mio provide that implementation? Or is there special handling per type of handle?

I was torn on this too, but I've chosen to introduce a trait because it can be extended in a source-compatible way by adding more trait methods with default impls if we find more cases of hooks that could be costomized by mio drivers (e.g. a "pre-poll hook").

There is a blanket impl for FnMuts as well, so they can be used, see the provided sample test case, which is doing exactly that.

Thomasdezeeuw · 2020-08-13T08:33:14Z

src/sys/windows/selector.rs

+        /// The callback will be called once the operation that it is bound to via a call to
+        /// `Overlapped::new` has been completed and the IOCP is signalled. By returning a `Readiness`
+        /// value, the callback may deliver a `mio::Event` to the user.
+        fn complete_operation(&mut self, entry: &OVERLAPPED_ENTRY) -> Option<Readiness>;


Personally not the biggest fan out exposing types from different crates, especially when those crates don't have a stable API.

I don't think that this happens here: This method resembles the old fn(&OVERLAPPED_ENTRY) from mio 0.6, so except from the OVERLAPPED_ENTRY (which is a stable Windows API struct) we do not expose other types. But maybe I misunderstand your comment.

Thomasdezeeuw · 2020-08-13T08:34:13Z

src/sys/windows/selector.rs

+        inner: UnsafeCell<miow::Overlapped>,
+        #[cfg(debug_assertions)]
+        canary: u32,
+        callback: Box<dyn CompletionCallback>,


Maybe fn(&OVERLAPPED_ENTRY) -> Option<Readiness> would be better, than we don't have to allocate.

This is correct. I've chosen to use a boxed trait here because I was looking at mio_named_pipes which does some pointer magic to get its state back inside the callback functions fore read, write and connect. By using dynamic dispatch, we can avoid that and have more safe code, but if resource usage beats that argument (plus the "future hooks" argument from above) I'd be willing to change this to a pure function pointer.

Thomasdezeeuw · 2020-08-13T08:35:45Z

src/sys/windows/selector.rs

+        ///
+        /// Won't actually do anything until `register_handle` has been called during a call to
+        /// `Registry::register`.
+        pub fn new(registry: &Registry, token: Token) -> Self {


Is there a reason why you don't require an event::Source on creation of Binding?

Actually I just need the handle, not the whole source. I split this into two parts as I wanted to give the user a chance to store the Binding instance before it starts firing events, so the usual sequence in a driver is:

Initialize the Option<Binding> member of your custom struct with None

Inside Source::register, you then set it to Some(Binding::new(..))

then* give it the handle to work with using register_handle

Thomasdezeeuw · 2020-08-13T08:36:42Z

src/sys/windows/selector.rs

+        /// Check whether the provided registry is the same as the one used during creation of the
+        /// binding.
+        ///
+        /// IOCP driven sources cannot unregister from their completion port before they get closed.


No event::Source in Mio can be registered with another Poll, so this is true for all platforms. See IoSource's selector_id field.

Ah, I see, so this is redundand and can be removed. I'll do that.

Thomasdezeeuw · 2020-08-13T08:37:41Z

src/sys/windows/selector.rs

+        /// completed IO operation, this method can be used to mark the associated token as ready, e.g.
+        /// in case it is necessary to signal an initial readiness before any IO operation has been
+        /// scheduled.
+        pub fn inject_event(&self, readiness: Readiness) {


I'm not sure about exposing such a function. Is the case you describe in the docs real? I personally never needed that.

While working on that PR, I used mio_named_pipes as a sample driver and forward ported it. There were cases (example: Establishing the first read after a successful connection) where in case of an error an immediate readiness flag was raised to be able to deliver the error. I also would love to get rid of that call as it requires a "side channel" for events but I couldn't think of a better solution yet.

Thomasdezeeuw · 2020-08-13T08:38:15Z

src/sys/windows/selector.rs

+        ///
+        /// This can be useful when only a shared borrow is held and the overlapped
+        /// pointer needs to be passed down to winapi.
+        pub fn as_mut_ptr(&self) -> *mut OVERLAPPED {


We don't really expose non-Mio types.

Ack, will change that one.

Thomasdezeeuw · 2020-08-17T17:42:14Z

@darkwisebear having though about this a bit more I think trying to SourceFd like API first would be the best way forward to make externally created event::Sources (e.g. unix/anonymous pipes). I don't know if that is possible however. If not lets try to minimise the API to begin with, e.g. only expose functions like inject_event once requested.

darkwisebear · 2020-08-17T19:08:42Z

@darkwisebear having though about this a bit more I think trying to SourceFd like API first would be the best way forward to make externally created event::Sources (e.g. unix/anonymous pipes). I don't know if that is possible however.

The major problem I see when trying to mimic SourceFd is that IOCP doesn't tell you what operation it was that finished when IOCPs get_many returns an event for a certain token (remember: IOCP is completion-based as opposed to readiness-based). Therefore we cannot (in the general case) deduce from a returned IOCP event what operation might now be available on a certain handle/source. I don't know what happens is if you simply signal read AND write readiness on event arrival, but that just doesn't sound right tbh :)

The only idea I have here is to provide premade OVERLAPPED factories that the user needs to use for the operations that require OVERLAPPEDs so that mio knows what kind of Event has to be generated. However, OVERLAPPEDs will also transport information like bytes transferred and the file offset back to the caller, information that is not available through the current mio event API - which I think is deliberate as other systems like async IO on Unix will return read/written bytes immediately, so this is a non-issue there. However, on Windows you usually at least need to know the number of bytes read/written to update internal buffers, maybe also file positions, depending on the driver.

All in all I'd say I agree with starting with a leaner API but I'm convinced that IOCP and SourceFd won't be a good match and we need a broader, more IOCP specific API like the one from mio 0.6.x that this PR is inspired by. In addition I'm not sure whether mio-named-pipes can be refactored such that this API will be sufficient. But if it turns out to be infeasible, we can still add the required APIs.

darkwisebear · 2020-10-05T10:29:06Z

@carllerche @Thomasdezeeuw Is this approach still relevant / desired in any form?

Thomasdezeeuw · 2020-10-05T17:46:59Z

@carllerche can you look at this? Seeing as an API akin to SourceFd doesn't seem possible it would require a bit more Windows knowledge.

Thomasdezeeuw · 2021-09-28T18:44:55Z

@darkwisebear do you plan on continuing this work?

darkwisebear · 2021-09-28T20:27:41Z

@Thomasdezeeuw Since there was no additional feedback I didn't pursue this PR any further but used an internal patched version of mio for my projects. Now there is tokio-rs/tokio#3760 which provides NamedPipe which was the reason I proposed this change. Are there other use cases for such an internal API and would it suffice/have a chance to be integrated like this?

Thomasdezeeuw · 2021-09-28T20:46:20Z

@Thomasdezeeuw Since there was no additional feedback I didn't pursue this PR any further but used an internal patched version of mio for my projects. Now there is tokio-rs/tokio#3760 which provides NamedPipe which was the reason I proposed this change. Are there other use cases for such an internal API and would it suffice/have a chance to be integrated like this?

I'm afraid I'm not familiar enough with Windows to answer that.

Also, sorry about the lack of feedback, I've been a bit busy over the last year or so, but I have a bit more time for Mio now.

darkwisebear · 2021-09-29T05:04:12Z

I'm afraid I'm not familiar enough with Windows to answer that.

I need to know two things:

Are there more use cases for the proposed API?
Is the API as well as the implementation (at least the aproach, the code itself code needs some love anyway since a lot of code has changed since then) close to a state that would lead to a final merge to master?

The core question now is: How/who can we answer those two questions? I would feel a lot more comfortable continuing to work if those were answered.

Also, sorry about the lack of feedback, I've been a bit busy over the last year or so, but I have a bit more time for Mio now.

No worries, it was meant as an offer to the community in case this was needed, didn't have strong feelings about merging that. If there is still need, we can continue working on it.

roblabla · 2021-09-29T09:13:37Z

Are there more use cases for the proposed API?

I can answer this one at least. I subscribed to this issue ages ago, because I was looking to integrate windows Filter Ports (FltGetMessage & co, used to communicate with drivers) into tokio. There are a few more APIs like this (most notably, ones based on DeviceIoControl) that currently cannot be integrated into mio cleanly.

Thomasdezeeuw · 2021-09-30T16:21:53Z

Are there more use cases for the proposed API?

I can answer this one at least. I subscribed to this issue ages ago, because I was looking to integrate windows Filter Ports (FltGetMessage & co, used to communicate with drivers) into tokio. There are a few more APIs like this (most notably, ones based on DeviceIoControl) that currently cannot be integrated into mio cleanly.

@roblabla could you maybe describe some (rough) requirement that you would need for Mio to work cleanly, and if this pr would (partially) implement those requirements.

Noah-Kennedy · 2021-10-03T17:21:19Z

I'm in a similar situation to @roblabla here.

Thomasdezeeuw · 2021-11-06T13:22:11Z

I'm closing this due to inactivity. @darkwisebear if you, or anyone else, want to continue this please don't feel discouraged to do so.

Thank you @darkwisebear for your efforts here.

Noah-Kennedy · 2021-11-11T22:08:11Z

@Thomasdezeeuw I have a use case where I need to support some custom sockets on windows. I would be willing to contribute to getting an API like this in place for mio.

Thomasdezeeuw · 2021-11-13T11:34:15Z

@Thomasdezeeuw I have a use case where I need to support some custom sockets on windows. I would be willing to contribute to getting an API like this in place for mio.

Sounds good. I can't offer much help with the implementation as I'm no Windows expert, but some do sometimes visit our Mio discord channel so you could ask for help there.

darkwisebear force-pushed the winapi-0.7 branch from a5b18bc to 61f3672 Compare August 11, 2020 06:24

darkwisebear added 2 commits August 11, 2020 20:51

Add API to hook into Windows IOCP loop

acfa2e4

An API similar to the one of mio 0.6 is now available so that custom objects can be added to the IOCP that is used to implement Poll.

Add preliminary test for the new old Windows IOCP API

bccc605

This is a first test to play with the Windows IOCP handler API.

darkwisebear force-pushed the winapi-0.7 branch from 61f3672 to bccc605 Compare August 11, 2020 18:52

Thomasdezeeuw reviewed Aug 13, 2020

View reviewed changes

Thomasdezeeuw requested a review from carllerche October 5, 2020 17:45

Thomasdezeeuw closed this Nov 6, 2021

Thomasdezeeuw mentioned this pull request Nov 6, 2021

windows: support custom completion port integrations #1047

Closed

Thomasdezeeuw mentioned this pull request Nov 13, 2021

Tracking issue for v1 #1533

Closed

Berrysoft mentioned this pull request Aug 26, 2023

Make CompletionPort public? #1714

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add API to hook into Windows IOCP loop #1345

Add API to hook into Windows IOCP loop #1345

darkwisebear commented Aug 11, 2020 •

edited

Thomasdezeeuw left a comment

Thomasdezeeuw Aug 13, 2020

darkwisebear Aug 13, 2020

Thomasdezeeuw Aug 13, 2020

Thomasdezeeuw Aug 13, 2020

darkwisebear Aug 13, 2020

Thomasdezeeuw Aug 13, 2020

darkwisebear Aug 13, 2020

Thomasdezeeuw Aug 13, 2020

darkwisebear Aug 13, 2020

Thomasdezeeuw Aug 13, 2020

darkwisebear Aug 13, 2020

Thomasdezeeuw Aug 13, 2020

darkwisebear Aug 13, 2020

Thomasdezeeuw Aug 13, 2020

darkwisebear Aug 13, 2020

Thomasdezeeuw Aug 13, 2020

darkwisebear Aug 13, 2020

Thomasdezeeuw commented Aug 17, 2020

darkwisebear commented Aug 17, 2020

darkwisebear commented Oct 5, 2020

Thomasdezeeuw commented Oct 5, 2020

Thomasdezeeuw commented Sep 28, 2021

darkwisebear commented Sep 28, 2021

Thomasdezeeuw commented Sep 28, 2021

darkwisebear commented Sep 29, 2021

roblabla commented Sep 29, 2021

Thomasdezeeuw commented Sep 30, 2021

Noah-Kennedy commented Oct 3, 2021

Thomasdezeeuw commented Nov 6, 2021

Noah-Kennedy commented Nov 11, 2021 •

edited

Thomasdezeeuw commented Nov 13, 2021

Add API to hook into Windows IOCP loop #1345

Add API to hook into Windows IOCP loop #1345

Conversation

darkwisebear commented Aug 11, 2020 • edited

Thomasdezeeuw left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Thomasdezeeuw commented Aug 17, 2020

darkwisebear commented Aug 17, 2020

darkwisebear commented Oct 5, 2020

Thomasdezeeuw commented Oct 5, 2020

Thomasdezeeuw commented Sep 28, 2021

darkwisebear commented Sep 28, 2021

Thomasdezeeuw commented Sep 28, 2021

darkwisebear commented Sep 29, 2021

roblabla commented Sep 29, 2021

Thomasdezeeuw commented Sep 30, 2021

Noah-Kennedy commented Oct 3, 2021

Thomasdezeeuw commented Nov 6, 2021

Noah-Kennedy commented Nov 11, 2021 • edited

Thomasdezeeuw commented Nov 13, 2021

darkwisebear commented Aug 11, 2020 •

edited

Noah-Kennedy commented Nov 11, 2021 •

edited