m(windows): Reimplement Wepoll in Rust

[notgull]

Thus far, wepoll has served us well in terms of polling on Windows. However, at this stage, it's time to migrate to our own home-rolled implementation. There is at least one critical soundness bug that stems from our usage of wepoll (#85), and it would be significantly easier to compile polling (as well as all of its dependents) if we purged our only C dependency. In addition, there are some issues that we can only solve if we move away from wepoll (smol-rs/async-io#17, smol-rs/async-io#25, #82).

The new strategy is similar to what wepoll currently does, except (hopefully) more robust. The core consists of using an I/O Completion Port as the core of the system, and then using the undocumented Auxillary Function Driver to deliver completion packets whenever a socket is considered "ready". There is a state object associated with each socket that contains the events currently being polled, the in-flight events associated with the socket, and whether the socket state is queued for deletion.

The differences between this implementation and wepoll includes:

• Rather than using a red-black tree to store socket state, we use a HashMap.
• A bounded ConcurrentQueue is used to hold pending socket updates, rather than a naive queue implementation. This queue has a capacity of 1024 elements and every update is run run the queue is full. In contrast, wepoll's queue is unbounded.
• Packets have Pin guarantees, in order to prevent state captured by the I/O completion port from being moved.
• Instead of going through the epoll_event intermediary, we directly convert completion packets to Events.
• We directly support notifying the Poller through a designated wakeup packet.
• We only expect one user to be polling at a time, so we use an AtomicBool instead of an atomic number.

Closes #22 and closes #85. Supersedes #76.

[notgull]

I think this is stable enough for review now; the tests aren't catching any bugs that I can see (although the precision tests are taking a while? But that might just be part of the test).

[taiki-e]

Thanks! I have not reviewed the details regarding the Windows API, but at first glance, this looks great.

Also, we may want to test some downstream crates before merging/releasing this patch because such a rewrite may cause subtle differences in behavior. (in the similar way that our CI tests async-io)

[notgull]

Also, we may want to test some downstream crates before merging/releasing this patch because such a rewrite may cause subtle differences in behavior.

I just ran a small "crater test" on a few crates that have polling in their dependency tree. I ran cargo test for:

• async-io
• async-net
• async-std
• isahc
• libp2p (had to figure out how to set up protoc on Windows for this 😮‍💨)
• quinn (tests mostly use tokio but the ones I modified to use async-std seemed to turn out OK)
• zbus

All of the tests seem to pass (or at least the ones that weren't already failing on Windows). I think we're fine in terms of potentially undefined behavior.

Edit: I included this excerpt in their Cargo.tomls:

[patch.crates-io]
polling = { git = 'https://github.com/smol-rs/polling', branch = 'notgull/reimplement-wepoll' }

[fogti]

why is the pointer that this function returns pinned?

[notgull]

The pinning ensures that the overlapped object isn't moved while it's involved in an operation.

[fogti]

how could it be moved when it is behind an & reference?

[notgull]

While it's being used in the overlapped operation, there is no reference (or at least, none that the borrow checker is aware of). As a safeguard against undefined behavior, this ensures that it isn't invalidated during the operation.

[fogti]

hm, seems a bit brittle, but I'll go with it for now.

[fogti]

Thanks for the work btw. Looks impressive.

[notgull]

Thanks for the work btw. Looks impressive.

Thanks for the review! I know this is a monster PR, so I appreciate you taking the time to sit down and look through it all. I'll merge once the CI passes.

[fogti]

It might be a good idea to do another downstream test, otherwise this looks ok. I think it is appropriate to squash-merge this when done. The possible brittleness of <Pin<Arc<T>> as CompletionHandle>::get() (or maybe the whole CompletionHandle + OVERLAPPED API) should imo be mentioned/appended to the intro post in the resulting merge commit.
"run run" in the intro post looks like a mistake btw.

We only expect one user to be polling at a time, so we use an AtomicBool instead of an atomic number.

Can/do we enforce that?

[notgull]

We only expect one user to be polling at a time, so we use an AtomicBool instead of an atomic number.

Can/do we enforce that?

A Mutex in the top-level Poller is locked before any polling is done.

polling/src/lib.rs

Lines 491 to 505 in e85331c

	if let Ok(mut lock) = self.events.try_lock() {
	// Wait for I/O events.
	self.poller.wait(&mut lock, timeout)?;
	// Clear the notification, if any.
	self.notified.swap(false, Ordering::SeqCst);
	// Collect events.
	let len = events.len();
	events.extend(lock.iter().filter(|ev| ev.key != usize::MAX));
	Ok(events.len() - len)
	} else {
	log::trace!("wait: skipping because another thread is already waiting on I/O");
	Ok(0)
	}