Support blocking for epoll

[tiif]

This PR enabled epoll to have blocking operation.

The changes introduced by this PR are:

• Refactored part of the logic in epoll_wait to blocking_epoll_callback
• Added a new field thread_ids in Epoll for blocked thread ids
• Added a new BlockReason::Epoll

[RalfJung]

place: &'tcx MPlaceTy<'tcx> is required to be annotated with 'tcx.

This should be place: MPlaceTy<'tcx>. (Use clone if needed.) I have no idea how you even got a reference with 'tcx lifetime...

EDIT: Ah, you don't. And it is indeed impossible, this approach is a dead end. Just using an owned MPlaceTy should fix that. :)

[bors]

☔ The latest upstream changes (presumably #3712) made this pull request unmergeable. Please resolve the merge conflicts.

[tiif]

Just using an owned MPlaceTy should fix that.

This works, thanks!

[bors]

☔ The latest upstream changes (presumably #3809) made this pull request unmergeable. Please resolve the merge conflicts.

[RalfJung]

I wonder if it would make sense to implement #3665 first, as that's a much easier case of blocking? That might be a good way to learn how Miri deals with blocking.

[oli-obk]

That's what I said, too, in our last meeting, and we realized it is not easier, as you need to refactor all read/write methods instead of just focussing on this one

[oli-obk]

(If those are done first, I'd like to have a PR that passes dest to r/w and writes directly from there instead of returning the io result.)

[RalfJung]

That's what I said, too, in our last meeting, and we realized it is not easier, as you need to refactor all read/write methods instead of just focussing on this one

Oh wait, so that does not have to happen for this one? Ah right, the blocking is in epoll_wait, not in read/write... makes sense.

[oli-obk]

Please bless and fix the tokio sleep test

[oli-obk]

could also just wake here and record in a hashset whether the thread id was already woken

[tiif]

For the new change in 9cb087d, I was trying to avoid using this.unblock_thread inside

if let Some(epoll_interests) = this.machine.epoll_interests.get_epoll_interest(id) { }

because it will trigger this error:

error[E0502]: cannot borrow `*this` as mutable because it is also borrowed as immutable
   --> src/shims/unix/linux/epoll.rs:594:29
    |
568 |         if let Some(epoll_interests) = this.machine.epoll_interests.get_epoll_interest(id) {
    |                                        ---------------------------- immutable borrow occurs here
...
573 |             for weak_epoll_interest in epoll_interests {
    |                                        --------------- immutable borrow later used here
...
594 |                             this.unblock_thread(thread_id, BlockReason::Epoll)?;
    |                             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ mutable borrow occurs here

So instead I just store the ThreadIds that I want to unblock into a Vec then unblock them altogether outside of the block.

[tiif]

This is how it currently works:
Edge-triggered notification (which is what we currently supporting) will only unblock one thread when a notification is received, even when multiple threads are blocked on the same epoll instance. So before we call this.block_thread, we record the ThreadId in Epoll file description using the thread_id field:

struct Epoll {
    /// A map of EpollEventInterests registered under this epoll instance.
    /// Each entry is differentiated using FdId and file descriptor value.
    interest_list: RefCell<BTreeMap<(FdId, i32), Rc<RefCell<EpollEventInterest>>>>,
    /// A map of EpollEventInstance that will be returned when `epoll_wait` is called.
    /// Similar to interest_list, the entry is also differentiated using FdId
    /// and file descriptor value.
    // This is an Rc because EpollInterest need to hold a reference to update
    // it.
    ready_list: Rc<RefCell<BTreeMap<(FdId, i32), EpollEventInstance>>>,
    /// A list of thread ids blocked on this epoll instance.
    thread_id: RefCell<Vec<ThreadId>>, // < this is newly added
}

Later in check_and_update_readiness, when we return a notification, we retrieve the list of blocked thread in that epoll instance, then unblock one of them.

[tiif]

@rustbot ready

[oli-obk]

The PR's main message is a bit outdated

[tiif]

Do we want another test for triggering a notification after the epoll_wait times out or is the sleep.rs test good enough?

[RalfJung]

There should be such a test in your libc-epoll-blocking test file. But doesn't `test_epoll_block_without_notification` do that?

[tiif]

But doesn't test_epoll_block_without_notification do that?

No, that test only epoll_wait, then timeout without notification. A notification need to be sent after timeout to trigger the bug. Also there is an error on the comment for that test, sorry for the confusion.

[RalfJung]

Okay, just make sure the test has a comment explaining the exact scenario. :)

[oli-obk]

@bors r+

[bors]

📌 Commit 2523c17 has been approved by oli-obk

It is now in the queue for this repository.

[bors]

⌛ Testing commit 2523c17 with merge 297482d...

[bors]

☀️ Test successful - checks-actions
Approved by: oli-obk
Pushing 297482d to master...