Remove the context API from epoll

[notgull]

It looks like we were overthinking it; from a brief glance at the documentation, it looks like deleting a file descriptor registered in epoll is not undefined behavior, at least by Rust's specifications. This PR removes the Context API and just uses the BorrowedFd API.

Closes #477

[sunfishcode]

This is a pretty subtle situation. It's true that there's no use-after-free hazard, but there is a use-of-a-borrow-after-it-goes-out-of-scope hazard. And I'm not sure how to think about that yet.

If some code does epoll.add(&listen_sock, 1, epoll::EventFlags::IN)?;, the & in this position in Rust typically means "borrow this for the duration of this call". But the semantics of epoll are such that, in this code, listen_sock is captured and may be continue to be referenced through the epoll object after the call. That's not something that regular borrowing in Rust can do, so it's not immediately clear that we want to allow that in I/O-safe APIs either.

[notgull]

From a semantic point of view, I like to think of a file descriptor as an Arc<dyn SystemResource>. And, in this call, the Arc is cloned. In this case, it would be valid under the Rust borrow system.

[sunfishcode]

The equivalent of an Arc clone is a dup, but EPOLL_CTL_ADD isn't a dup, and I expect this is observable in various ways. For example, events will contain the file descriptors' numbers, so it's observable that they "escaped" the borrow without a dup.

We may be able to do this, but I want to think through the consequences more.

[notgull]

The events themselves actually don't contain the file descriptor; it's just the event number that was passed in to the event.

(This may be an issue for kqueue, however.)

[sunfishcode]

Rustix's epoll API currently does store the file descriptor number in the epoll user data, and provides it to users as a file descriptor along with events. This way users that just need the file descriptor can get it, without having to keep a separate HashMap or using raw file descriptors and unsafe.

That said, I have been thinking about whether it's better to just letting users manage the data field, as this PR does, even if it does push people towards unsafe in some use cases. The extra flexibility and simplicity may be worth it.

[notgull]

I replaced Epoll with OwnedFd, as mentioned in #488 (comment)

[notgull]

@sunfishcode Is there any chance you can review this?

Rustix's epoll API currently does store the file descriptor number in the epoll user data, and provides it to users as a file descriptor along with events. This way users that just need the file descriptor can get it, without having to keep a separate HashMap or using raw file descriptors and unsafe.

In my opinion, this abstraction is too high level for rustix. If users want to keep track of data associated with the data field, it should be their decision to do so.

[sunfishcode]

In my opinion, this abstraction is too high level for rustix. If users want to keep track of data associated with the data field, it should be their decision to do so.

I agree. I think this PR sounds like the right approach overall. The current Epoll abstraction was an attempt to provide an API that wouldn't require users to use unsafe, but it ended up getting too complex. This does appear to be a disadvantageous abstraction level for safety.

I'm still concerned about epoll holding onto file descriptors and reporting events for them after the scope of their borrow ends. There's no direct use-after-close hazard, but it seems error-prone. What would you think about making epoll_add unsafe, having it take an AsRawFd, and documentingbthat it behaves as if the raw fd is retained?

Also, tagging with 0.37, as this is a semver bump.

[notgull]

I'm still concerned about epoll holding onto file descriptors and reporting events for them after the scope of their borrow ends. There's no direct use-after-close hazard, but it seems error-prone. What would you think about making epoll_add unsafe, having it take an AsRawFd, and documentingbthat it behaves as if the raw fd is retained?

I don't think there's much danger here. The worst thing that can happen is a spurious event (which is pretty much expected to happen with leaky behavior like this anyways). I've added documentation to epoll_add indicating this, but I don't personally think it's enough to mark this as unsafe.

[sunfishcode]

I started a thread about this in Zulip: https://rust-lang.zulipchat.com/#narrow/stream/219381-t-libs/topic/I.2FO.20safety.20for.20epoll . @bjorn3 suggested thinking of epoll as doing a "weak dup", which I'm thinking makes sense. If you want to write some documentation along those lines feel free, though I'd be happy to do a follow-up PR myself.

[sunfishcode]

This needs a rustfmt for.CI, and I notice one other simplification we can make now:

[notgull]

If you want to write some documentation along those lines feel free, though I'd be happy to do a follow-up PR myself.

I added a mention of it to epoll_add, but you may want to elaborate on it.

[sunfishcode]

Sounds good!