int: Use polling crate as interface to poll system

[notgull]

Replaces the sys submodules with the polling crate.

[ids1024]

Using polling, is there a way to subscribe to non-fd kqueue event types as suggested in https://github.com/Smithay/calloop/issues/116, or would that be impossible with this implementation?

As we discussed on https://github.com/Smithay/calloop/pull/119, the current EventSource API could violate IO safety with poll, but not with epoll/kqueue which hold the reference to the file in kernel. Though I guess that's mainly a technical issue with bad EventSource implementations, and using RawFd it's already trivial to violate IO safety from safe code anyway.

Anyway, this should provide Windows support, through wepoll? I'm not sure exactly what the implications of that are vs. direct use of native IOCP. If Windows support is added this should probably be documented and have a CI test.

[notgull]

Using polling, is there a way to subscribe to non-fd kqueue event types as suggested in #116, or would that be impossible with this implementation?

Good idea, I've opened smol-rs/polling#68 to address this.

As we discussed on #119, the current EventSource API could violate IO safety with poll, but not with epoll/kqueue which hold the reference to the file in kernel. Though I guess that's mainly a technical issue with bad EventSource implementations, and using RawFd it's already trivial to violate IO safety from safe code anyway.

I've done some testing with this. The poll() implementation might be IO unsafe in this way, but the rest aren't. In any case, we're planning on migrating to I/O safe primitives once Debian stable supports it. See smol-rs/polling#38

Anyway, this should provide Windows support, through wepoll? I'm not sure exactly what the implications of that are vs. direct use of native IOCP. If Windows support is added this should probably be documented and have a CI test.

True, I'll add it later. I seem to have broken Linux, so I need to go back and figure out how to fix that.

The main thing to keep in mind when using wepoll is that in addition to ICOP is uses \Device\Afd, an unstable, undocumented Windows API. This is mostly just something to be aware of, as it isn't really an issue in practice (for instance, Node.JS uses the same unstable API). However, it has caused issues in the past; see tokio-rs/mio#1444.

[ids1024]

For IO safety, with https://github.com/Smithay/calloop/pull/119 this runs into issues since register/unregister are passed a BorrowedFd, so the poller shouldn't be able to keep a file descriptor (to call poll on). I think some kind of change to calloop's EventSource API would be necessary to handle this correctly.

Ah, so polling uses wepoll, while mio uses the same technique as wepoll but implemented in Rust? So the advantages and caveats should be similar other than requiring a C compiler.

[notgull]

For IO safety, with #119 this runs into issues since register/unregister are passed a BorrowedFd, so the poller shouldn't be able to keep a file descriptor (to call poll on).

I mean, in theory, we could just dup the file descriptor.

Ah, so polling uses epoll, while mio uses the same technique as wepoll but implemented in Rust? So the advantages and caveats should be similar other than requiring a C compiler.

Yes. I've been planning on rewriting the Windows backend for polling from scratch, actually; the wepoll dependency is mildly annoying.

[codecov[bot]]

Codecov Report

Patch coverage: 98.59% and project coverage change: +0.28 :tada:

Comparison is base (cba4d3a) 87.89% compared to head (439a547) 88.17%.

Additional details and impacted files

```diff @@ Coverage Diff @@ ## master #123 +/- ## ========================================== + Coverage 87.89% 88.17% +0.28% ========================================== Files 14 15 +1 Lines 1553 1556 +3 ========================================== + Hits 1365 1372 +7 + Misses 188 184 -4 ``` | Flag | Coverage Δ | | |---|---|---| | macos-latest | `87.77% <98.59%> (-0.12%)` | :arrow_down: | | ubuntu-latest | `87.70% <98.59%> (?)` | | Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Smithay#carryforward-flags-in-the-pull-request-comment) to find out more. | [Impacted Files](https://codecov.io/gh/Smithay/calloop/pull/123?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Smithay) | Coverage Δ | | |---|---|---| | [src/error.rs](https://codecov.io/gh/Smithay/calloop/pull/123?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Smithay#diff-c3JjL2Vycm9yLnJz) | `0.00% <ø> (ø)` | | | [src/sources/timer.rs](https://codecov.io/gh/Smithay/calloop/pull/123?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Smithay#diff-c3JjL3NvdXJjZXMvdGltZXIucnM=) | `86.53% <ø> (ø)` | | | [src/sys.rs](https://codecov.io/gh/Smithay/calloop/pull/123?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Smithay#diff-c3JjL3N5cy5ycw==) | `98.41% <98.41%> (ø)` | | | [src/io.rs](https://codecov.io/gh/Smithay/calloop/pull/123?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Smithay#diff-c3JjL2lvLnJz) | `82.75% <100.00%> (+0.09%)` | :arrow_up: | | [src/loop\_logic.rs](https://codecov.io/gh/Smithay/calloop/pull/123?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Smithay#diff-c3JjL2xvb3BfbG9naWMucnM=) | `90.45% <100.00%> (-1.73%)` | :arrow_down: | ... and [3 files with indirect coverage changes](https://codecov.io/gh/Smithay/calloop/pull/123/indirect-changes?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Smithay) Help us with your feedback. Take ten seconds to tell us [how you rate us](https://about.codecov.io/nps?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Smithay). Have a feature suggestion? [Share it here.](https://app.codecov.io/gh/feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Smithay)

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Do you have feedback about the report comment? Let us know in this issue.

[notgull]

I won't enable Windows support in CI yet, since I've stumbled into an open design decision regarding the ping event sources. On Windows, there's no real equivalent to an eventfd or a Unix pair pipe. You can use a TCP-self pipe, but that has a tendency to break.

polling gets around this by posting a null-notification to the IOCP and then interpreting that as a wakeup. I'm not sure if this strategy is possible with the ping source's multiple token values.

[Drakulix]

I mean, in theory, we could just dup the file descriptor.

Please don't. Doing so can have implications for resource tracking on the drm-subsystem and might make it harder to figure out if every fd has been properly set to be CLOEXEC, which is super important for smithay, where we use this library.

That said I applaud the effort put in to add multi-platform support. :+1:

[notgull]

I realized I probably marked this as ready prematurely, since polling hasn't released smol-rs/polling#59 yet.

[elinorbgr]

Ah, I merged #119 without thinking about this PR, sorry for that. I hope this is not too big of a problem?

[notgull]

Don't worry, it doesn't intersect much with this PR at all. Besides, this one probably won't be ready for a while.

[notgull]

polling version 2.6 has been released! The new version not only contains new polling modes, but also uses a new, no-C-dependency Windows backend but also provides handles for handling signals on kqueue. I think this is ready for review now.

[ids1024]

At least with a bad EventSource implementation, an fd could be registered, closed and possibly re-used, and later used in calls to poll, right? As was mentioned in https://github.com/Smithay/calloop/pull/119 this technically wasn't an issue with just epoll/kqueue backends.

Technically this allows safe code to cause a violation of IO safety rules, though I'm not sure if polling alone would cause soundness issues. I guess it would some sort of (not necessarily unsound) problems if the fd is reused and registered again as a different EventSource?

[ids1024]

If we're confident that won't cause soundness issues and also doesn't really introduce more weird bugs with bad implementations of EventSource beyond what's already possible, that doesn't need to block merging this PR. But it's worth considering if we can change the calloop API to ensure that's impossible with safe code.

[notgull]

It's not unsound yet; in the future, I/O safety may be checked by MIRI and other tools. But that would require an ecosystem overhaul that's years out. This isn't an issue for the time being.

[ids1024]

True, that could produce MIRI errors if MIRI some day has a concept of fd providence and handles poll in some way. (If it doesn't already; haven't tried running calloop in miri). But in actual use the impact of calling poll from libc is in itself opaque to the compiler, if it doesn't impact the behavior of other calls operating on the same fd, so that shouldn't actually cause soundness issues in the future.

But yeah, either way something that could be improved but shouldn't be a real issue.

[elinorbgr]

Okay, looks good now, thanks!