From 2f0793284ee9ecbb3bac8a2cdc341aaec966fcbf Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Sun, 9 Nov 2025 17:02:43 +0100 Subject: [PATCH 1/2] epoll: unblock threads when new interest is registered --- src/shims/files.rs | 20 ++ src/shims/unix/linux_like/epoll.rs | 202 ++++++++---------- src/shims/unix/linux_like/eventfd.rs | 4 +- src/shims/unix/unnamed_socket.rs | 6 +- tests/deps/Cargo.lock | 96 +++++++++ tests/deps/Cargo.toml | 1 + tests/fail-dep/libc/libc-epoll-data-race.rs | 2 +- tests/pass-dep/libc/libc-epoll-blocking.rs | 39 ++++ tests/pass-dep/libc/libc-epoll-no-blocking.rs | 11 +- 9 files changed, 259 insertions(+), 122 deletions(-) diff --git a/src/shims/files.rs b/src/shims/files.rs index 8c29cb040b..22c2e3d083 100644 --- a/src/shims/files.rs +++ b/src/shims/files.rs @@ -50,6 +50,26 @@ impl FileDescriptionRef { } } +impl PartialEq for FileDescriptionRef { + fn eq(&self, other: &Self) -> bool { + self.id() == other.id() + } +} + +impl Eq for FileDescriptionRef {} + +impl PartialOrd for FileDescriptionRef { + fn partial_cmp(&self, other: &Self) -> Option { + Some(self.cmp(other)) + } +} + +impl Ord for FileDescriptionRef { + fn cmp(&self, other: &Self) -> std::cmp::Ordering { + self.id().cmp(&other.id()) + } +} + /// Holds a weak reference to the actual file description. #[derive(Debug)] pub struct WeakFileDescriptionRef(Weak>); diff --git a/src/shims/unix/linux_like/epoll.rs b/src/shims/unix/linux_like/epoll.rs index 3133c14929..4958cdfa5d 100644 --- a/src/shims/unix/linux_like/epoll.rs +++ b/src/shims/unix/linux_like/epoll.rs @@ -1,5 +1,5 @@ use std::cell::RefCell; -use std::collections::BTreeMap; +use std::collections::{BTreeMap, BTreeSet}; use std::io; use std::rc::{Rc, Weak}; use std::time::Duration; @@ -18,11 +18,13 @@ use crate::*; struct Epoll { /// A map of EpollEventInterests registered under this epoll instance. /// Each entry is differentiated using FdId and file descriptor value. + /// The interest are separately refcounted so that we can track, for each FD, what epoll + /// instances are currently interested in it. interest_list: RefCell>>>, /// 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. - ready_list: ReadyList, + ready_list: RefCell>, /// A list of thread ids blocked on this epoll instance. blocked_tid: RefCell>, } @@ -36,9 +38,9 @@ impl VisitProvenance for Epoll { /// EpollEventInstance contains information that will be returned by epoll_wait. #[derive(Debug)] pub struct EpollEventInstance { - /// Xor-ed event types that happened to the file description. + /// Bitmask of event types that happened to the file description. events: u32, - /// Original data retrieved from `epoll_event` during `epoll_ctl`. + /// User-defined data associated with the interest that triggered this instance. data: u64, /// The release clock associated with this event. clock: VClock, @@ -99,11 +101,6 @@ pub struct EpollReadyEvents { pub epollerr: bool, } -#[derive(Debug, Default)] -struct ReadyList { - mapping: RefCell>, -} - impl EpollReadyEvents { pub fn new() -> Self { EpollReadyEvents { @@ -185,8 +182,8 @@ impl EpollInterestTable { } } - pub fn get_epoll_interest(&self, id: FdId) -> Option<&Vec>>> { - self.0.get(&id) + pub fn get_epoll_interest(&self, id: FdId) -> Option<&[Weak>]> { + self.0.get(&id).map(|v| &**v) } pub fn get_epoll_interest_mut( @@ -328,18 +325,7 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> { let epoll_key = (id, fd); - // Check the existence of fd in the interest list. - if op == epoll_ctl_add { - if interest_list.contains_key(&epoll_key) { - return this.set_last_error_and_return_i32(LibcError("EEXIST")); - } - } else { - if !interest_list.contains_key(&epoll_key) { - return this.set_last_error_and_return_i32(LibcError("ENOENT")); - } - } - - if op == epoll_ctl_add { + let new_interest = if op == epoll_ctl_add { // Create an epoll_interest. let interest = Rc::new(RefCell::new(EpollEventInterest { fd_num: fd, @@ -347,24 +333,34 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> { data, weak_epfd: FileDescriptionRef::downgrade(&epfd), })); - // Notification will be returned for current epfd if there is event in the file - // descriptor we registered. - check_and_update_one_event_interest(&fd_ref, &interest, id, this)?; - - // Insert an epoll_interest to global epoll_interest list. + // Register it in the right places. this.machine.epoll_interests.insert_epoll_interest(id, Rc::downgrade(&interest)); - interest_list.insert(epoll_key, interest); + if interest_list.insert(epoll_key, interest.clone()).is_some() { + // We already had interest in this. + return this.set_last_error_and_return_i32(LibcError("EEXIST")); + } + + interest } else { // Modify the existing interest. - let epoll_interest = interest_list.get_mut(&epoll_key).unwrap(); + let Some(interest) = interest_list.get_mut(&epoll_key) else { + return this.set_last_error_and_return_i32(LibcError("ENOENT")); + }; { - let mut epoll_interest = epoll_interest.borrow_mut(); - epoll_interest.events = events; - epoll_interest.data = data; + let mut interest = interest.borrow_mut(); + interest.events = events; + interest.data = data; } - // Updating an FD interest triggers events. - check_and_update_one_event_interest(&fd_ref, epoll_interest, id, this)?; - } + interest.clone() + }; + + // Deliver events for the new interest. + send_ready_events_to_interests( + this, + fd_ref.as_unix(this).get_epoll_ready_events()?.get_event_bitmask(this), + id, + std::iter::once(new_interest), + )?; interp_ok(Scalar::from_i32(0)) } else if op == epoll_ctl_del { @@ -378,7 +374,7 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> { drop(epoll_interest); // Remove related epoll_interest from ready list. - epfd.ready_list.mapping.borrow_mut().remove(&epoll_key); + epfd.ready_list.borrow_mut().remove(&epoll_key); // Remove dangling EpollEventInterest from its global table. // .unwrap() below should succeed because the file description id must have registered @@ -462,7 +458,7 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> { }; // We just need to know if the ready list is empty and borrow the thread_ids out. - let ready_list_empty = epfd.ready_list.mapping.borrow().is_empty(); + let ready_list_empty = epfd.ready_list.borrow().is_empty(); if timeout == 0 || !ready_list_empty { // If the ready list is not empty, or the timeout is 0, we can return immediately. return_ready_list(&epfd, dest, &event, this)?; @@ -518,50 +514,71 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> { interp_ok(()) } - /// For a specific file description, get its ready events and update the corresponding ready - /// list. This function should be called whenever an event causes more bytes or an EOF to become - /// newly readable from an FD, and whenever more bytes can be written to an FD or no more future - /// writes are possible. + /// For a specific file description, get its ready events and send it to everyone who registered + /// interest in this FD. This function should be called whenever an event causes more bytes or + /// an EOF to become newly readable from an FD, and whenever more bytes can be written to an FD + /// or no more future writes are possible. /// /// This *will* report an event if anyone is subscribed to it, without any further filtering, so /// do not call this function when an FD didn't have anything happen to it! - fn check_and_update_readiness( - &mut self, - fd_ref: DynFileDescriptionRef, - ) -> InterpResult<'tcx, ()> { + fn epoll_send_fd_ready_events(&mut self, fd_ref: DynFileDescriptionRef) -> InterpResult<'tcx> { let this = self.eval_context_mut(); + let ready_events_bitmask = + fd_ref.as_unix(this).get_epoll_ready_events()?.get_event_bitmask(this); let id = fd_ref.id(); - let mut waiter = Vec::new(); - // Get a list of EpollEventInterest that is associated to a specific file description. - if let Some(epoll_interests) = this.machine.epoll_interests.get_epoll_interest(id) { - for weak_epoll_interest in epoll_interests { - if let Some(epoll_interest) = weak_epoll_interest.upgrade() { - let is_updated = - check_and_update_one_event_interest(&fd_ref, &epoll_interest, id, this)?; - if is_updated { - // Edge-triggered notification only notify one thread even if there are - // multiple threads blocked on the same epfd. - - // This unwrap can never fail because if the current epoll instance were - // closed, the upgrade of weak_epoll_interest - // above would fail. This guarantee holds because only the epoll instance - // holds a strong ref to epoll_interest. - let epfd = epoll_interest.borrow().weak_epfd.upgrade().unwrap(); - // FIXME: We can randomly pick a thread to unblock. - if let Some(thread_id) = epfd.blocked_tid.borrow_mut().pop() { - waiter.push(thread_id); - }; - } - } - } + // Figure out who is interested in this. We need to clone this list since we can't prove + // that `send_ready_events_to_interests` won't mutate it. + let interests = this.machine.epoll_interests.get_epoll_interest(id).unwrap_or(&[]); + let interests = interests.iter().filter_map(|weak| weak.upgrade()).collect::>(); + send_ready_events_to_interests(this, ready_events_bitmask, id, interests.into_iter()) + } +} + +/// Send the latest ready events for one particular FD (identified by `event_fd_id`) to everyone in +/// the `interests` list, if they are interested in this kind of event. +fn send_ready_events_to_interests<'tcx>( + ecx: &mut MiriInterpCx<'tcx>, + event_bitmask: u32, + event_fd_id: FdId, + interests: impl Iterator>>, +) -> InterpResult<'tcx> { + #[allow(clippy::mutable_key_type)] // yeah, we know + let mut triggered_epolls = BTreeSet::new(); + for interest in interests { + let interest = interest.borrow(); + let epfd = interest.weak_epfd.upgrade().unwrap(); + // This checks if any of the events specified in epoll_event_interest.events + // match those in ready_events. + let flags = interest.events & event_bitmask; + if flags == 0 { + continue; } - waiter.sort(); - waiter.dedup(); - for thread_id in waiter { - this.unblock_thread(thread_id, BlockReason::Epoll)?; + // Geenrate a new event instance, with the flags that this one is interested in. + let mut new_instance = EpollEventInstance::new(flags, interest.data); + ecx.release_clock(|clock| { + new_instance.clock.clone_from(clock); + })?; + // Add event to ready list for this epoll instance. + // Tests confirm that we have to *overwrite* the old instance for the same key. + let mut ready_list = epfd.ready_list.borrow_mut(); + ready_list.insert((event_fd_id, interest.fd_num), new_instance); + drop(ready_list); + // Remember to wake up this epoll later. + // (We might encounter the same epoll multiple times if there are multiple interests for + // different file descriptors that references the same file description.) + triggered_epolls.insert(epfd); + } + + // For each epoll instance where an interest triggered, wake up one thread. + for epoll in triggered_epolls { + // Edge-triggered notification only notify one thread even if there are + // multiple threads blocked on the same epfd. + if let Some(thread_id) = epoll.blocked_tid.borrow_mut().pop() { + ecx.unblock_thread(thread_id, BlockReason::Epoll)?; } - interp_ok(()) } + + interp_ok(()) } /// This function takes in ready list and returns EpollEventInstance with file description @@ -582,41 +599,6 @@ fn ready_list_next( None } -/// This helper function checks whether an epoll notification should be triggered for a specific -/// epoll_interest and, if necessary, triggers the notification, and returns whether the -/// notification was added/updated. Unlike check_and_update_readiness, this function sends a -/// notification to only one epoll instance. -fn check_and_update_one_event_interest<'tcx>( - fd_ref: &DynFileDescriptionRef, - interest: &RefCell, - id: FdId, - ecx: &MiriInterpCx<'tcx>, -) -> InterpResult<'tcx, bool> { - // Get the bitmask of ready events for a file description. - let ready_events_bitmask = fd_ref.as_unix(ecx).get_epoll_ready_events()?.get_event_bitmask(ecx); - let epoll_event_interest = interest.borrow(); - let epfd = epoll_event_interest.weak_epfd.upgrade().unwrap(); - // This checks if any of the events specified in epoll_event_interest.events - // match those in ready_events. - let flags = epoll_event_interest.events & ready_events_bitmask; - // If there is any event that we are interested in being specified as ready, - // insert an epoll_return to the ready list. - if flags != 0 { - let epoll_key = (id, epoll_event_interest.fd_num); - let mut ready_list = epfd.ready_list.mapping.borrow_mut(); - let mut event_instance = EpollEventInstance::new(flags, epoll_event_interest.data); - // If we are tracking data races, remember the current clock so we can sync with it later. - ecx.release_clock(|clock| { - event_instance.clock.clone_from(clock); - })?; - // Triggers the notification by inserting it to the ready list. - ready_list.insert(epoll_key, event_instance); - interp_ok(true) - } else { - interp_ok(false) - } -} - /// Stores the ready list of the `epfd` epoll instance into `events` (which must be an array), /// and the number of returned events into `dest`. fn return_ready_list<'tcx>( @@ -625,7 +607,7 @@ fn return_ready_list<'tcx>( events: &MPlaceTy<'tcx>, ecx: &mut MiriInterpCx<'tcx>, ) -> InterpResult<'tcx> { - let mut ready_list = epfd.ready_list.mapping.borrow_mut(); + let mut ready_list = epfd.ready_list.borrow_mut(); let mut num_of_events: i32 = 0; let mut array_iter = ecx.project_array_fields(events)?; diff --git a/src/shims/unix/linux_like/eventfd.rs b/src/shims/unix/linux_like/eventfd.rs index 5d4f207d36..ea68698df2 100644 --- a/src/shims/unix/linux_like/eventfd.rs +++ b/src/shims/unix/linux_like/eventfd.rs @@ -216,7 +216,7 @@ fn eventfd_write<'tcx>( // The state changed; we check and update the status of all supported event // types for current file description. - ecx.check_and_update_readiness(eventfd)?; + ecx.epoll_send_fd_ready_events(eventfd)?; // Return how many bytes we consumed from the user-provided buffer. return finish.call(ecx, Ok(buf_place.layout.size.bytes_usize())); @@ -311,7 +311,7 @@ fn eventfd_read<'tcx>( // The state changed; we check and update the status of all supported event // types for current file description. - ecx.check_and_update_readiness(eventfd)?; + ecx.epoll_send_fd_ready_events(eventfd)?; // Tell userspace how many bytes we put into the buffer. return finish.call(ecx, Ok(buf_place.layout.size.bytes_usize())); diff --git a/src/shims/unix/unnamed_socket.rs b/src/shims/unix/unnamed_socket.rs index 81703d6e17..d0a5ba911b 100644 --- a/src/shims/unix/unnamed_socket.rs +++ b/src/shims/unix/unnamed_socket.rs @@ -96,7 +96,7 @@ impl FileDescription for AnonSocket { } } // Notify peer fd that close has happened, since that can unblock reads and writes. - ecx.check_and_update_readiness(peer_fd)?; + ecx.epoll_send_fd_ready_events(peer_fd)?; } interp_ok(Ok(())) } @@ -276,7 +276,7 @@ fn anonsocket_write<'tcx>( } // Notification should be provided for peer fd as it became readable. // The kernel does this even if the fd was already readable before, so we follow suit. - ecx.check_and_update_readiness(peer_fd)?; + ecx.epoll_send_fd_ready_events(peer_fd)?; return finish.call(ecx, Ok(write_size)); } @@ -369,7 +369,7 @@ fn anonsocket_read<'tcx>( ecx.unblock_thread(thread_id, BlockReason::UnnamedSocket)?; } // Notify epoll waiters. - ecx.check_and_update_readiness(peer_fd)?; + ecx.epoll_send_fd_ready_events(peer_fd)?; }; return finish.call(ecx, Ok(read_size)); diff --git a/tests/deps/Cargo.lock b/tests/deps/Cargo.lock index 65ca4215c6..187411588e 100644 --- a/tests/deps/Cargo.lock +++ b/tests/deps/Cargo.lock @@ -72,6 +72,95 @@ version = "2.3.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "37909eebbb50d72f9059c3b6d82c0463f2ff062c9e95845c43a6c9c0355411be" +[[package]] +name = "futures" +version = "0.3.31" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "65bc07b1a8bc7c85c5f2e110c476c7389b4554ba72af57d8445ea63a576b0876" +dependencies = [ + "futures-channel", + "futures-core", + "futures-executor", + "futures-io", + "futures-sink", + "futures-task", + "futures-util", +] + +[[package]] +name = "futures-channel" +version = "0.3.31" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "2dff15bf788c671c1934e366d07e30c1814a8ef514e1af724a602e8a2fbe1b10" +dependencies = [ + "futures-core", + "futures-sink", +] + +[[package]] +name = "futures-core" +version = "0.3.31" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "05f29059c0c2090612e8d742178b0580d2dc940c837851ad723096f87af6663e" + +[[package]] +name = "futures-executor" +version = "0.3.31" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "1e28d1d997f585e54aebc3f97d39e72338912123a67330d723fdbb564d646c9f" +dependencies = [ + "futures-core", + "futures-task", + "futures-util", +] + +[[package]] +name = "futures-io" +version = "0.3.31" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "9e5c1b78ca4aae1ac06c48a526a655760685149f0d465d21f37abfe57ce075c6" + +[[package]] +name = "futures-macro" +version = "0.3.31" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "162ee34ebcb7c64a8abebc059ce0fee27c2262618d7b60ed8faf72fef13c3650" +dependencies = [ + "proc-macro2", + "quote", + "syn", +] + +[[package]] +name = "futures-sink" +version = "0.3.31" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "e575fab7d1e0dcb8d0c7bcf9a63ee213816ab51902e6d244a95819acacf1d4f7" + +[[package]] +name = "futures-task" +version = "0.3.31" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "f90f7dce0722e95104fcb095585910c0977252f286e354b5e3bd38902cd99988" + +[[package]] +name = "futures-util" +version = "0.3.31" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "9fa08315bb612088cc391249efdc3bc77536f16c91f6cf495e6fbe85b20a4a81" +dependencies = [ + "futures-channel", + "futures-core", + "futures-io", + "futures-macro", + "futures-sink", + "futures-task", + "memchr", + "pin-project-lite", + "pin-utils", + "slab", +] + [[package]] name = "getrandom" version = "0.1.16" @@ -190,6 +279,7 @@ name = "miri-test-deps" version = "0.1.0" dependencies = [ "cfg-if", + "futures", "getrandom 0.1.16", "getrandom 0.2.16", "getrandom 0.3.3", @@ -242,6 +332,12 @@ version = "0.2.16" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "3b3cff922bd51709b605d9ead9aa71031d81447142d828eb4a6eba76fe619f9b" +[[package]] +name = "pin-utils" +version = "0.1.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "8b870d8c151b6f2fb93e84a13146138f05d02ed11c7e7c54f8826aaaf7c9f184" + [[package]] name = "proc-macro2" version = "1.0.95" diff --git a/tests/deps/Cargo.toml b/tests/deps/Cargo.toml index d85723f091..fe1586280b 100644 --- a/tests/deps/Cargo.toml +++ b/tests/deps/Cargo.toml @@ -23,6 +23,7 @@ page_size = "0.6" # Avoid pulling in all of tokio's dependencies. # However, without `net` and `signal`, tokio uses fewer relevant system APIs. tokio = { version = "1", features = ["macros", "rt-multi-thread", "time", "net", "fs", "sync", "signal", "io-util"] } +futures = { version = "0.3.0", features = ["async-await"] } [target.'cfg(windows)'.dependencies] windows-sys = { version = "0.60", features = [ diff --git a/tests/fail-dep/libc/libc-epoll-data-race.rs b/tests/fail-dep/libc/libc-epoll-data-race.rs index f6ec5be61b..bca886d375 100644 --- a/tests/fail-dep/libc/libc-epoll-data-race.rs +++ b/tests/fail-dep/libc/libc-epoll-data-race.rs @@ -32,7 +32,7 @@ fn check_epoll_wait(epfd: i32, expected_notifications: &[(u32, u for (return_event, expected_event) in slice.iter().zip(expected_notifications.iter()) { let event = return_event.events; let data = return_event.u64; - assert_eq!(event, expected_event.0, "got wrong events"); + assert_eq!(event, expected_event.0, "got wrong events bitmask"); assert_eq!(data, expected_event.1, "got wrong data"); } } diff --git a/tests/pass-dep/libc/libc-epoll-blocking.rs b/tests/pass-dep/libc/libc-epoll-blocking.rs index c97206487a..6609dd6e25 100644 --- a/tests/pass-dep/libc/libc-epoll-blocking.rs +++ b/tests/pass-dep/libc/libc-epoll-blocking.rs @@ -16,6 +16,7 @@ fn main() { test_epoll_block_then_unblock(); test_notification_after_timeout(); test_epoll_race(); + wakeup_on_new_interest(); } // Using `as` cast since `EPOLLET` wraps around @@ -179,3 +180,41 @@ fn test_epoll_race() { }; thread1.join().unwrap(); } + +/// Ensure that a blocked thread gets woken up when new interested are registered with the +/// epoll it is blocked on. +fn wakeup_on_new_interest() { + // Create an epoll instance. + let epfd = unsafe { libc::epoll_create1(0) }; + assert_ne!(epfd, -1); + + // Create a socketpair instance. + let mut fds = [-1, -1]; + let res = unsafe { libc::socketpair(libc::AF_UNIX, libc::SOCK_STREAM, 0, fds.as_mut_ptr()) }; + assert_eq!(res, 0); + + // Write to fd[0] + let data = "abcde".as_bytes().as_ptr(); + let res = unsafe { libc_utils::write_all(fds[0], data as *const libc::c_void, 5) }; + assert_eq!(res, 5); + + // Block a thread on the epoll instance. + let t = std::thread::spawn(move || { + let expected_event = u32::try_from(libc::EPOLLIN | libc::EPOLLOUT).unwrap(); + let expected_value = u64::try_from(fds[1]).unwrap(); + check_epoll_wait::<8>(epfd, &[(expected_event, expected_value)], -1); + }); + // Ensure the thread is blocked. + std::thread::yield_now(); + + // Register fd[1] with EPOLLIN|EPOLLOUT|EPOLLET|EPOLLRDHUP + let mut ev = libc::epoll_event { + events: (libc::EPOLLIN | libc::EPOLLOUT | libc::EPOLLET | libc::EPOLLRDHUP) as _, + u64: u64::try_from(fds[1]).unwrap(), + }; + let res = unsafe { libc::epoll_ctl(epfd, libc::EPOLL_CTL_ADD, fds[1], &mut ev) }; + assert_eq!(res, 0); + + // This should wake up the thread. + t.join().unwrap(); +} diff --git a/tests/pass-dep/libc/libc-epoll-no-blocking.rs b/tests/pass-dep/libc/libc-epoll-no-blocking.rs index 7130790b86..01dfbbfaae 100644 --- a/tests/pass-dep/libc/libc-epoll-no-blocking.rs +++ b/tests/pass-dep/libc/libc-epoll-no-blocking.rs @@ -70,16 +70,16 @@ fn test_epoll_socketpair() { let res = unsafe { libc_utils::write_all(fds[0], data as *const libc::c_void, 5) }; assert_eq!(res, 5); - // Register fd[1] with EPOLLIN|EPOLLOUT|EPOLLET|EPOLLRDHUP + // Register fd[1] with EPOLLOUT|EPOLLET|EPOLLRDHUP but NOT EPOLLIN let mut ev = libc::epoll_event { - events: (libc::EPOLLIN | libc::EPOLLOUT | libc::EPOLLET | libc::EPOLLRDHUP) as _, + events: (libc::EPOLLOUT | libc::EPOLLET | libc::EPOLLRDHUP) as _, u64: u64::try_from(fds[1]).unwrap(), }; let res = unsafe { libc::epoll_ctl(epfd, libc::EPOLL_CTL_ADD, fds[1], &mut ev) }; assert_eq!(res, 0); - // Check result from epoll_wait. - let expected_event = u32::try_from(libc::EPOLLIN | libc::EPOLLOUT).unwrap(); + // Check result from epoll_wait. EPOLLIN should be masked away. + let expected_event = u32::try_from(libc::EPOLLOUT).unwrap(); let expected_value = u64::try_from(fds[1]).unwrap(); check_epoll_wait::<8>(epfd, &[(expected_event, expected_value)]); @@ -101,8 +101,7 @@ fn test_epoll_socketpair() { // Check result from epoll_wait. // We expect to get a read, write, HUP notification from the close since closing an FD always unblocks reads and writes on its peer. - let expected_event = - u32::try_from(libc::EPOLLRDHUP | libc::EPOLLIN | libc::EPOLLOUT | libc::EPOLLHUP).unwrap(); + let expected_event = u32::try_from(libc::EPOLLRDHUP | libc::EPOLLOUT | libc::EPOLLHUP).unwrap(); let expected_value = u64::try_from(fds[1]).unwrap(); check_epoll_wait::<8>(epfd, &[(expected_event, expected_value)]); } From 852446e017840ca5975b028f1f55943800681dde Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Sun, 9 Nov 2025 18:42:49 +0100 Subject: [PATCH 2/2] simplify epoll data structures: dont make the interests themselves into refcounted objects --- src/shims/files.rs | 46 ++-- src/shims/unix/linux_like/epoll.rs | 245 +++++++++--------- src/shims/unix/linux_like/eventfd.rs | 3 +- src/shims/unix/unnamed_socket.rs | 3 +- src/shims/windows/fs.rs | 6 +- tests/deps/Cargo.lock | 96 ------- tests/deps/Cargo.toml | 1 - tests/fail-dep/libc/libc-epoll-data-race.rs | 11 +- tests/pass-dep/libc/libc-epoll-blocking.rs | 16 +- tests/pass-dep/libc/libc-epoll-no-blocking.rs | 83 +++--- 10 files changed, 191 insertions(+), 319 deletions(-) diff --git a/src/shims/files.rs b/src/shims/files.rs index 22c2e3d083..ea19c2d501 100644 --- a/src/shims/files.rs +++ b/src/shims/files.rs @@ -14,7 +14,7 @@ use crate::*; /// A unique id for file descriptions. While we could use the address, considering that /// is definitely unique, the address would expose interpreter internal state when used -/// for sorting things. So instead we generate a unique id per file description is the name +/// for sorting things. So instead we generate a unique id per file description which is the same /// for all `dup`licates and is never reused. #[derive(Debug, Copy, Clone, Default, Eq, PartialEq, Ord, PartialOrd)] pub struct FdId(usize); @@ -48,25 +48,11 @@ impl FileDescriptionRef { pub fn id(&self) -> FdId { self.0.id } -} - -impl PartialEq for FileDescriptionRef { - fn eq(&self, other: &Self) -> bool { - self.id() == other.id() - } -} - -impl Eq for FileDescriptionRef {} -impl PartialOrd for FileDescriptionRef { - fn partial_cmp(&self, other: &Self) -> Option { - Some(self.cmp(other)) - } -} - -impl Ord for FileDescriptionRef { - fn cmp(&self, other: &Self) -> std::cmp::Ordering { - self.id().cmp(&other.id()) + /// Returns the raw address of this file description. Useful for equality comparisons. + /// Use `id` instead if this can affect user-visible behavior! + pub fn addr(&self) -> usize { + Rc::as_ptr(&self.0).addr() } } @@ -90,6 +76,11 @@ impl WeakFileDescriptionRef { pub fn upgrade(&self) -> Option> { self.0.upgrade().map(FileDescriptionRef) } + + /// Returns the raw address of this file description. Useful for equality comparisons. + pub fn addr(&self) -> usize { + self.0.as_ptr().addr() + } } impl VisitProvenance for WeakFileDescriptionRef { @@ -125,12 +116,13 @@ impl FileDescriptionExt for T { communicate_allowed: bool, ecx: &mut MiriInterpCx<'tcx>, ) -> InterpResult<'tcx, io::Result<()>> { + let addr = self.addr(); match Rc::into_inner(self.0) { Some(fd) => { - // Remove entry from the global epoll_event_interest table. - ecx.machine.epoll_interests.remove(fd.id); + // There might have been epolls interested in this FD. Remove that. + ecx.machine.epoll_interests.remove_epolls(fd.id); - fd.inner.close(communicate_allowed, ecx) + fd.inner.destroy(addr, communicate_allowed, ecx) } None => { // Not the last reference. @@ -203,9 +195,12 @@ pub trait FileDescription: std::fmt::Debug + FileDescriptionExt { throw_unsup_format!("cannot seek on {}", self.name()); } - /// Close the file descriptor. - fn close<'tcx>( + /// Destroys the file description. Only called when the last duplicate file descriptor is closed. + /// + /// `self_addr` is the address that this file description used to be stored at. + fn destroy<'tcx>( self, + _self_addr: usize, _communicate_allowed: bool, _ecx: &mut MiriInterpCx<'tcx>, ) -> InterpResult<'tcx, io::Result<()>> @@ -382,8 +377,9 @@ impl FileDescription for FileHandle { interp_ok((&mut &self.file).seek(offset)) } - fn close<'tcx>( + fn destroy<'tcx>( self, + _self_addr: usize, communicate_allowed: bool, _ecx: &mut MiriInterpCx<'tcx>, ) -> InterpResult<'tcx, io::Result<()>> { diff --git a/src/shims/unix/linux_like/epoll.rs b/src/shims/unix/linux_like/epoll.rs index 4958cdfa5d..d832d10782 100644 --- a/src/shims/unix/linux_like/epoll.rs +++ b/src/shims/unix/linux_like/epoll.rs @@ -1,30 +1,33 @@ use std::cell::RefCell; -use std::collections::{BTreeMap, BTreeSet}; +use std::collections::BTreeMap; use std::io; -use std::rc::{Rc, Weak}; use std::time::Duration; use rustc_abi::FieldIdx; use crate::concurrency::VClock; use crate::shims::files::{ - DynFileDescriptionRef, FdId, FileDescription, FileDescriptionRef, WeakFileDescriptionRef, + DynFileDescriptionRef, FdId, FdNum, FileDescription, FileDescriptionRef, WeakFileDescriptionRef, }; use crate::shims::unix::UnixFileDescription; use crate::*; +type EpollEventKey = (FdId, FdNum); + /// An `Epoll` file descriptor connects file handles and epoll events #[derive(Debug, Default)] struct Epoll { /// A map of EpollEventInterests registered under this epoll instance. /// Each entry is differentiated using FdId and file descriptor value. - /// The interest are separately refcounted so that we can track, for each FD, what epoll - /// instances are currently interested in it. - interest_list: RefCell>>>, + interest_list: RefCell>, /// 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. - ready_list: RefCell>, + /// We keep this separate from `interest_list` for two reasons: there might be many + /// interests but only a few of them ready (so with a separate list it is more efficient + /// to find a ready event), and having separate `RefCell` lets us mutate the `interest_list` + /// while unblocking threads which might mutate the `ready_list`. + ready_list: RefCell>, /// A list of thread ids blocked on this epoll instance. blocked_tid: RefCell>, } @@ -35,6 +38,11 @@ impl VisitProvenance for Epoll { } } +/// Returns the range of all EpollEventKey for the given FD ID. +fn range_for_id(id: FdId) -> std::ops::RangeInclusive { + (id, 0)..=(id, i32::MAX) +} + /// EpollEventInstance contains information that will be returned by epoll_wait. #[derive(Debug)] pub struct EpollEventInstance { @@ -61,13 +69,8 @@ impl EpollEventInstance { /// see the man page: /// /// -#[derive(Debug)] +#[derive(Debug, Copy, Clone)] pub struct EpollEventInterest { - /// The file descriptor value of the file description registered. - /// This is only used for ready_list, to inform userspace which FD triggered an event. - /// For that, it is crucial to preserve the original FD number. - /// This FD number must never be "dereferenced" to a file description inside Miri. - fd_num: i32, /// The events bitmask retrieved from `epoll_event`. events: u32, /// The data retrieved from `epoll_event`. @@ -75,13 +78,10 @@ pub struct EpollEventInterest { /// but only u64 is supported for now. /// data: u64, - /// The epoll file description that this EpollEventInterest is registered under. - /// This is weak to avoid cycles, but an upgrade is always guaranteed to succeed - /// because only the `Epoll` holds a strong ref to a `EpollEventInterest`. - weak_epfd: WeakFileDescriptionRef, } /// EpollReadyEvents reflects the readiness of a file description. +#[derive(Debug)] pub struct EpollReadyEvents { /// The associated file is available for read(2) operations, in the sense that a read will not block. /// (I.e., returning EOF is considered "ready".) @@ -144,11 +144,18 @@ impl FileDescription for Epoll { "epoll" } - fn close<'tcx>( - self, + fn destroy<'tcx>( + mut self, + self_addr: usize, _communicate_allowed: bool, - _ecx: &mut MiriInterpCx<'tcx>, + ecx: &mut MiriInterpCx<'tcx>, ) -> InterpResult<'tcx, io::Result<()>> { + // If we were interested in some FDs, we can remove that now. + let mut ids = self.interest_list.get_mut().keys().map(|(id, _num)| *id).collect::>(); + ids.dedup(); // they come out of the map sorted + for id in ids { + ecx.machine.epoll_interests.remove(id, self_addr); + } interp_ok(Ok(())) } @@ -160,41 +167,54 @@ impl FileDescription for Epoll { impl UnixFileDescription for Epoll {} /// The table of all EpollEventInterest. -/// The BTreeMap key is the FdId of an active file description registered with -/// any epoll instance. The value is a list of EpollEventInterest associated -/// with that file description. -pub struct EpollInterestTable(BTreeMap>>>); +/// This tracks, for each file description, which epoll instances have an interest in events +/// for this file description. +pub struct EpollInterestTable(BTreeMap>>); impl EpollInterestTable { pub(crate) fn new() -> Self { EpollInterestTable(BTreeMap::new()) } - pub fn insert_epoll_interest(&mut self, id: FdId, fd: Weak>) { - match self.0.get_mut(&id) { - Some(fds) => { - fds.push(fd); - } - None => { - let vec = vec![fd]; - self.0.insert(id, vec); - } - } + fn insert(&mut self, id: FdId, epoll: WeakFileDescriptionRef) { + let epolls = self.0.entry(id).or_default(); + epolls.push(epoll); } - pub fn get_epoll_interest(&self, id: FdId) -> Option<&[Weak>]> { - self.0.get(&id).map(|v| &**v) + fn remove(&mut self, id: FdId, epoll_addr: usize) { + let epolls = self.0.entry(id).or_default(); + // FIXME: linear scan. Keep the list sorted so we can do binary search? + let idx = epolls + .iter() + .position(|old_ref| old_ref.addr() == epoll_addr) + .expect("trying to remove an epoll that's not in the list"); + epolls.remove(idx); } - pub fn get_epoll_interest_mut( - &mut self, - id: FdId, - ) -> Option<&mut Vec>>> { - self.0.get_mut(&id) + fn get_epolls(&self, id: FdId) -> Option<&Vec>> { + self.0.get(&id) } - pub fn remove(&mut self, id: FdId) { - self.0.remove(&id); + pub fn remove_epolls(&mut self, id: FdId) { + if let Some(epolls) = self.0.remove(&id) { + for epoll in epolls.iter().filter_map(|e| e.upgrade()) { + // This is a still-live epoll with interest in this FD. Remove all + // relevent interests. + epoll + .interest_list + .borrow_mut() + .extract_if(range_for_id(id), |_, _| true) + // Consume the iterator. + .for_each(|_| ()); + // Also remove all events from the ready list that refer to this FD. + epoll + .ready_list + .borrow_mut() + .extract_if(range_for_id(id), |_, _| true) + // Consume the iterator. + .for_each(|_| ()); + } + } } } @@ -261,9 +281,9 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> { let epollhup = this.eval_libc_u32("EPOLLHUP"); let epollerr = this.eval_libc_u32("EPOLLERR"); - // Throw EINVAL if epfd and fd have the same value. + // Throw EFAULT if epfd and fd have the same value. if epfd_value == fd { - return this.set_last_error_and_return_i32(LibcError("EINVAL")); + return this.set_last_error_and_return_i32(LibcError("EFAULT")); } // Check if epfd is a valid epoll file descriptor. @@ -323,43 +343,33 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> { ); } + // Add new interest to list. let epoll_key = (id, fd); - - let new_interest = if op == epoll_ctl_add { - // Create an epoll_interest. - let interest = Rc::new(RefCell::new(EpollEventInterest { - fd_num: fd, - events, - data, - weak_epfd: FileDescriptionRef::downgrade(&epfd), - })); - // Register it in the right places. - this.machine.epoll_interests.insert_epoll_interest(id, Rc::downgrade(&interest)); - if interest_list.insert(epoll_key, interest.clone()).is_some() { + let new_interest = EpollEventInterest { events, data }; + if op == epoll_ctl_add { + if interest_list.range(range_for_id(id)).next().is_none() { + // This is the first time this FD got added to this epoll. + // Remember that in the global list so we get notified about FD events. + this.machine.epoll_interests.insert(id, FileDescriptionRef::downgrade(&epfd)); + } + if interest_list.insert(epoll_key, new_interest).is_some() { // We already had interest in this. return this.set_last_error_and_return_i32(LibcError("EEXIST")); } - - interest } else { // Modify the existing interest. let Some(interest) = interest_list.get_mut(&epoll_key) else { return this.set_last_error_and_return_i32(LibcError("ENOENT")); }; - { - let mut interest = interest.borrow_mut(); - interest.events = events; - interest.data = data; - } - interest.clone() + *interest = new_interest; }; // Deliver events for the new interest. send_ready_events_to_interests( this, + &epfd, fd_ref.as_unix(this).get_epoll_ready_events()?.get_event_bitmask(this), - id, - std::iter::once(new_interest), + std::iter::once((&epoll_key, &new_interest)), )?; interp_ok(Scalar::from_i32(0)) @@ -367,25 +377,19 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> { let epoll_key = (id, fd); // Remove epoll_event_interest from interest_list. - let Some(epoll_interest) = interest_list.remove(&epoll_key) else { + if interest_list.remove(&epoll_key).is_none() { + // We did not have interest in this. return this.set_last_error_and_return_i32(LibcError("ENOENT")); }; - // All related Weak will fail to upgrade after the drop. - drop(epoll_interest); + // If this was the last interest in this FD, remove us from the global list + // of who is interested in this FD. + if interest_list.range(range_for_id(id)).next().is_none() { + this.machine.epoll_interests.remove(id, epfd.addr()); + } // Remove related epoll_interest from ready list. epfd.ready_list.borrow_mut().remove(&epoll_key); - // Remove dangling EpollEventInterest from its global table. - // .unwrap() below should succeed because the file description id must have registered - // at least one epoll_interest, if not, it will fail when removing epoll_interest from - // interest list. - this.machine - .epoll_interests - .get_epoll_interest_mut(id) - .unwrap() - .retain(|event| event.upgrade().is_some()); - interp_ok(Scalar::from_i32(0)) } else { throw_unsup_format!("unsupported epoll_ctl operation: {op}"); @@ -523,30 +527,43 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> { /// do not call this function when an FD didn't have anything happen to it! fn epoll_send_fd_ready_events(&mut self, fd_ref: DynFileDescriptionRef) -> InterpResult<'tcx> { let this = self.eval_context_mut(); - let ready_events_bitmask = - fd_ref.as_unix(this).get_epoll_ready_events()?.get_event_bitmask(this); let id = fd_ref.id(); // Figure out who is interested in this. We need to clone this list since we can't prove - // that `send_ready_events_to_interests` won't mutate it. - let interests = this.machine.epoll_interests.get_epoll_interest(id).unwrap_or(&[]); - let interests = interests.iter().filter_map(|weak| weak.upgrade()).collect::>(); - send_ready_events_to_interests(this, ready_events_bitmask, id, interests.into_iter()) + // that `send_ready_events_to_interest` won't mutate it. + let Some(epolls) = this.machine.epoll_interests.get_epolls(id) else { + return interp_ok(()); + }; + let epolls = epolls + .iter() + .map(|weak| { + weak.upgrade() + .expect("someone forgot to remove the garbage from `machine.epoll_interests`") + }) + .collect::>(); + let event_bitmask = fd_ref.as_unix(this).get_epoll_ready_events()?.get_event_bitmask(this); + for epoll in epolls { + send_ready_events_to_interests( + this, + &epoll, + event_bitmask, + epoll.interest_list.borrow().range(range_for_id(id)), + )?; + } + + interp_ok(()) } } -/// Send the latest ready events for one particular FD (identified by `event_fd_id`) to everyone in +/// Send the latest ready events for one particular FD (identified by `event_key`) to everyone in /// the `interests` list, if they are interested in this kind of event. -fn send_ready_events_to_interests<'tcx>( +fn send_ready_events_to_interests<'tcx, 'a>( ecx: &mut MiriInterpCx<'tcx>, + epoll: &Epoll, event_bitmask: u32, - event_fd_id: FdId, - interests: impl Iterator>>, + interests: impl Iterator, ) -> InterpResult<'tcx> { - #[allow(clippy::mutable_key_type)] // yeah, we know - let mut triggered_epolls = BTreeSet::new(); - for interest in interests { - let interest = interest.borrow(); - let epfd = interest.weak_epfd.upgrade().unwrap(); + let mut wakeup = false; + for (&event_key, interest) in interests { // This checks if any of the events specified in epoll_event_interest.events // match those in ready_events. let flags = interest.events & event_bitmask; @@ -560,19 +577,15 @@ fn send_ready_events_to_interests<'tcx>( })?; // Add event to ready list for this epoll instance. // Tests confirm that we have to *overwrite* the old instance for the same key. - let mut ready_list = epfd.ready_list.borrow_mut(); - ready_list.insert((event_fd_id, interest.fd_num), new_instance); - drop(ready_list); - // Remember to wake up this epoll later. - // (We might encounter the same epoll multiple times if there are multiple interests for - // different file descriptors that references the same file description.) - triggered_epolls.insert(epfd); + let mut ready_list = epoll.ready_list.borrow_mut(); + ready_list.insert(event_key, new_instance); + wakeup = true; } - - // For each epoll instance where an interest triggered, wake up one thread. - for epoll in triggered_epolls { + if wakeup { + // Wake up threads that may have been waiting for events on this epoll. + // Do this only once for all the interests. // Edge-triggered notification only notify one thread even if there are - // multiple threads blocked on the same epfd. + // multiple threads blocked on the same epoll. if let Some(thread_id) = epoll.blocked_tid.borrow_mut().pop() { ecx.unblock_thread(thread_id, BlockReason::Epoll)?; } @@ -581,24 +594,6 @@ fn send_ready_events_to_interests<'tcx>( interp_ok(()) } -/// This function takes in ready list and returns EpollEventInstance with file description -/// that is not closed. -fn ready_list_next( - ecx: &MiriInterpCx<'_>, - ready_list: &mut BTreeMap<(FdId, i32), EpollEventInstance>, -) -> Option { - while let Some((epoll_key, epoll_event_instance)) = ready_list.pop_first() { - // This ensures that we only return events that we are interested. The FD might have been closed since - // the event was generated, in which case we are not interested anymore. - // When a file description is fully closed, it gets removed from `machine.epoll_interests`, - // so we skip events whose FD is not in that map anymore. - if ecx.machine.epoll_interests.get_epoll_interest(epoll_key.0).is_some() { - return Some(epoll_event_instance); - } - } - None -} - /// Stores the ready list of the `epfd` epoll instance into `events` (which must be an array), /// and the number of returned events into `dest`. fn return_ready_list<'tcx>( @@ -612,7 +607,7 @@ fn return_ready_list<'tcx>( let mut array_iter = ecx.project_array_fields(events)?; while let Some(des) = array_iter.next(ecx)? { - if let Some(epoll_event_instance) = ready_list_next(ecx, &mut ready_list) { + if let Some((_, epoll_event_instance)) = ready_list.pop_first() { ecx.write_int_fields_named( &[ ("events", epoll_event_instance.events.into()), diff --git a/src/shims/unix/linux_like/eventfd.rs b/src/shims/unix/linux_like/eventfd.rs index ea68698df2..ce51f25793 100644 --- a/src/shims/unix/linux_like/eventfd.rs +++ b/src/shims/unix/linux_like/eventfd.rs @@ -37,8 +37,9 @@ impl FileDescription for EventFd { "event" } - fn close<'tcx>( + fn destroy<'tcx>( self, + _self_addr: usize, _communicate_allowed: bool, _ecx: &mut MiriInterpCx<'tcx>, ) -> InterpResult<'tcx, io::Result<()>> { diff --git a/src/shims/unix/unnamed_socket.rs b/src/shims/unix/unnamed_socket.rs index d0a5ba911b..cfd453df26 100644 --- a/src/shims/unix/unnamed_socket.rs +++ b/src/shims/unix/unnamed_socket.rs @@ -82,8 +82,9 @@ impl FileDescription for AnonSocket { } } - fn close<'tcx>( + fn destroy<'tcx>( self, + _self_addr: usize, _communicate_allowed: bool, ecx: &mut MiriInterpCx<'tcx>, ) -> InterpResult<'tcx, io::Result<()>> { diff --git a/src/shims/windows/fs.rs b/src/shims/windows/fs.rs index e4ec1b0130..936bbf06eb 100644 --- a/src/shims/windows/fs.rs +++ b/src/shims/windows/fs.rs @@ -24,8 +24,9 @@ impl FileDescription for DirHandle { interp_ok(self.path.metadata()) } - fn close<'tcx>( + fn destroy<'tcx>( self, + _self_addr: usize, _communicate_allowed: bool, _ecx: &mut MiriInterpCx<'tcx>, ) -> InterpResult<'tcx, io::Result<()>> { @@ -50,8 +51,9 @@ impl FileDescription for MetadataHandle { interp_ok(Ok(self.meta.clone())) } - fn close<'tcx>( + fn destroy<'tcx>( self, + _self_addr: usize, _communicate_allowed: bool, _ecx: &mut MiriInterpCx<'tcx>, ) -> InterpResult<'tcx, io::Result<()>> { diff --git a/tests/deps/Cargo.lock b/tests/deps/Cargo.lock index 187411588e..65ca4215c6 100644 --- a/tests/deps/Cargo.lock +++ b/tests/deps/Cargo.lock @@ -72,95 +72,6 @@ version = "2.3.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "37909eebbb50d72f9059c3b6d82c0463f2ff062c9e95845c43a6c9c0355411be" -[[package]] -name = "futures" -version = "0.3.31" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "65bc07b1a8bc7c85c5f2e110c476c7389b4554ba72af57d8445ea63a576b0876" -dependencies = [ - "futures-channel", - "futures-core", - "futures-executor", - "futures-io", - "futures-sink", - "futures-task", - "futures-util", -] - -[[package]] -name = "futures-channel" -version = "0.3.31" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "2dff15bf788c671c1934e366d07e30c1814a8ef514e1af724a602e8a2fbe1b10" -dependencies = [ - "futures-core", - "futures-sink", -] - -[[package]] -name = "futures-core" -version = "0.3.31" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "05f29059c0c2090612e8d742178b0580d2dc940c837851ad723096f87af6663e" - -[[package]] -name = "futures-executor" -version = "0.3.31" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "1e28d1d997f585e54aebc3f97d39e72338912123a67330d723fdbb564d646c9f" -dependencies = [ - "futures-core", - "futures-task", - "futures-util", -] - -[[package]] -name = "futures-io" -version = "0.3.31" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "9e5c1b78ca4aae1ac06c48a526a655760685149f0d465d21f37abfe57ce075c6" - -[[package]] -name = "futures-macro" -version = "0.3.31" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "162ee34ebcb7c64a8abebc059ce0fee27c2262618d7b60ed8faf72fef13c3650" -dependencies = [ - "proc-macro2", - "quote", - "syn", -] - -[[package]] -name = "futures-sink" -version = "0.3.31" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "e575fab7d1e0dcb8d0c7bcf9a63ee213816ab51902e6d244a95819acacf1d4f7" - -[[package]] -name = "futures-task" -version = "0.3.31" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "f90f7dce0722e95104fcb095585910c0977252f286e354b5e3bd38902cd99988" - -[[package]] -name = "futures-util" -version = "0.3.31" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "9fa08315bb612088cc391249efdc3bc77536f16c91f6cf495e6fbe85b20a4a81" -dependencies = [ - "futures-channel", - "futures-core", - "futures-io", - "futures-macro", - "futures-sink", - "futures-task", - "memchr", - "pin-project-lite", - "pin-utils", - "slab", -] - [[package]] name = "getrandom" version = "0.1.16" @@ -279,7 +190,6 @@ name = "miri-test-deps" version = "0.1.0" dependencies = [ "cfg-if", - "futures", "getrandom 0.1.16", "getrandom 0.2.16", "getrandom 0.3.3", @@ -332,12 +242,6 @@ version = "0.2.16" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "3b3cff922bd51709b605d9ead9aa71031d81447142d828eb4a6eba76fe619f9b" -[[package]] -name = "pin-utils" -version = "0.1.0" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "8b870d8c151b6f2fb93e84a13146138f05d02ed11c7e7c54f8826aaaf7c9f184" - [[package]] name = "proc-macro2" version = "1.0.95" diff --git a/tests/deps/Cargo.toml b/tests/deps/Cargo.toml index fe1586280b..d85723f091 100644 --- a/tests/deps/Cargo.toml +++ b/tests/deps/Cargo.toml @@ -23,7 +23,6 @@ page_size = "0.6" # Avoid pulling in all of tokio's dependencies. # However, without `net` and `signal`, tokio uses fewer relevant system APIs. tokio = { version = "1", features = ["macros", "rt-multi-thread", "time", "net", "fs", "sync", "signal", "io-util"] } -futures = { version = "0.3.0", features = ["async-await"] } [target.'cfg(windows)'.dependencies] windows-sys = { version = "0.60", features = [ diff --git a/tests/fail-dep/libc/libc-epoll-data-race.rs b/tests/fail-dep/libc/libc-epoll-data-race.rs index bca886d375..3b1217cda1 100644 --- a/tests/fail-dep/libc/libc-epoll-data-race.rs +++ b/tests/fail-dep/libc/libc-epoll-data-race.rs @@ -28,13 +28,10 @@ fn check_epoll_wait(epfd: i32, expected_notifications: &[(u32, u expected_notifications.len().try_into().unwrap(), "got wrong number of notifications" ); - let slice = unsafe { std::slice::from_raw_parts(array_ptr, res.try_into().unwrap()) }; - for (return_event, expected_event) in slice.iter().zip(expected_notifications.iter()) { - let event = return_event.events; - let data = return_event.u64; - assert_eq!(event, expected_event.0, "got wrong events bitmask"); - assert_eq!(data, expected_event.1, "got wrong data"); - } + let got_notifications = + unsafe { std::slice::from_raw_parts(array_ptr, res.try_into().unwrap()) }; + let got_notifications = got_notifications.iter().map(|e| (e.events, e.u64)).collect::>(); + assert_eq!(got_notifications, expected_notifications, "got wrong notifications"); } fn main() { diff --git a/tests/pass-dep/libc/libc-epoll-blocking.rs b/tests/pass-dep/libc/libc-epoll-blocking.rs index 6609dd6e25..81d4e501c4 100644 --- a/tests/pass-dep/libc/libc-epoll-blocking.rs +++ b/tests/pass-dep/libc/libc-epoll-blocking.rs @@ -36,18 +36,10 @@ fn check_epoll_wait( if res < 0 { panic!("epoll_wait failed: {}", std::io::Error::last_os_error()); } - assert_eq!( - res, - expected_notifications.len().try_into().unwrap(), - "got wrong number of notifications" - ); - let slice = unsafe { std::slice::from_raw_parts(array_ptr, res.try_into().unwrap()) }; - for (return_event, expected_event) in slice.iter().zip(expected_notifications.iter()) { - let event = return_event.events; - let data = return_event.u64; - assert_eq!(event, expected_event.0, "got wrong events"); - assert_eq!(data, expected_event.1, "got wrong data"); - } + let got_notifications = + unsafe { std::slice::from_raw_parts(array_ptr, res.try_into().unwrap()) }; + let got_notifications = got_notifications.iter().map(|e| (e.events, e.u64)).collect::>(); + assert_eq!(got_notifications, expected_notifications, "got wrong notifications"); } // This test allows epoll_wait to block, then unblock without notification. diff --git a/tests/pass-dep/libc/libc-epoll-no-blocking.rs b/tests/pass-dep/libc/libc-epoll-no-blocking.rs index 01dfbbfaae..c39192766d 100644 --- a/tests/pass-dep/libc/libc-epoll-no-blocking.rs +++ b/tests/pass-dep/libc/libc-epoll-no-blocking.rs @@ -41,18 +41,10 @@ fn check_epoll_wait(epfd: i32, expected_notifications: &[(u32, u if res < 0 { panic!("epoll_wait failed: {}", std::io::Error::last_os_error()); } - assert_eq!( - res, - expected_notifications.len().try_into().unwrap(), - "got wrong number of notifications" - ); - let slice = unsafe { std::slice::from_raw_parts(array_ptr, res.try_into().unwrap()) }; - for (return_event, expected_event) in slice.iter().zip(expected_notifications.iter()) { - let event = return_event.events; - let data = return_event.u64; - assert_eq!(event, expected_event.0, "got wrong events"); - assert_eq!(data, expected_event.1, "got wrong data"); - } + let got_notifications = + unsafe { std::slice::from_raw_parts(array_ptr, res.try_into().unwrap()) }; + let got_notifications = got_notifications.iter().map(|e| (e.events, e.u64)).collect::>(); + assert_eq!(got_notifications, expected_notifications, "got wrong notifications"); } fn test_epoll_socketpair() { @@ -70,16 +62,16 @@ fn test_epoll_socketpair() { let res = unsafe { libc_utils::write_all(fds[0], data as *const libc::c_void, 5) }; assert_eq!(res, 5); - // Register fd[1] with EPOLLOUT|EPOLLET|EPOLLRDHUP but NOT EPOLLIN + // Register fd[1] with EPOLLIN|EPOLLOUT|EPOLLET|EPOLLRDHUP let mut ev = libc::epoll_event { - events: (libc::EPOLLOUT | libc::EPOLLET | libc::EPOLLRDHUP) as _, + events: (libc::EPOLLIN | libc::EPOLLOUT | libc::EPOLLET | libc::EPOLLRDHUP) as _, u64: u64::try_from(fds[1]).unwrap(), }; let res = unsafe { libc::epoll_ctl(epfd, libc::EPOLL_CTL_ADD, fds[1], &mut ev) }; assert_eq!(res, 0); - // Check result from epoll_wait. EPOLLIN should be masked away. - let expected_event = u32::try_from(libc::EPOLLOUT).unwrap(); + // Check result from epoll_wait. + let expected_event = u32::try_from(libc::EPOLLIN | libc::EPOLLOUT).unwrap(); let expected_value = u64::try_from(fds[1]).unwrap(); check_epoll_wait::<8>(epfd, &[(expected_event, expected_value)]); @@ -101,7 +93,8 @@ fn test_epoll_socketpair() { // Check result from epoll_wait. // We expect to get a read, write, HUP notification from the close since closing an FD always unblocks reads and writes on its peer. - let expected_event = u32::try_from(libc::EPOLLRDHUP | libc::EPOLLOUT | libc::EPOLLHUP).unwrap(); + let expected_event = + u32::try_from(libc::EPOLLIN | libc::EPOLLOUT | libc::EPOLLHUP | libc::EPOLLRDHUP).unwrap(); let expected_value = u64::try_from(fds[1]).unwrap(); check_epoll_wait::<8>(epfd, &[(expected_event, expected_value)]); } @@ -229,7 +222,7 @@ fn test_two_same_fd_in_same_epoll_instance() { let res = unsafe { libc_utils::write_all(fds[0], data as *const libc::c_void, 5) }; assert_eq!(res, 5); - //Two notification should be received. + // Two notification should be received. let expected_event = u32::try_from(libc::EPOLLIN | libc::EPOLLOUT).unwrap(); let expected_value = 5 as u64; check_epoll_wait::<8>( @@ -287,7 +280,7 @@ fn test_epoll_socketpair_both_sides() { let res = unsafe { libc_utils::write_all(fds[1], data as *const libc::c_void, 5) }; assert_eq!(res, 5); - //Two notification should be received. + // Two notification should be received. let expected_event0 = u32::try_from(libc::EPOLLIN | libc::EPOLLOUT).unwrap(); let expected_value0 = fds[0] as u64; let expected_event1 = u32::try_from(libc::EPOLLOUT).unwrap(); @@ -447,12 +440,13 @@ fn test_socketpair_read() { let res = unsafe { libc::epoll_ctl(epfd, libc::EPOLL_CTL_ADD, fds[1], &mut ev) }; assert_eq!(res, 0); - // Write 5 bytes to fds[1]. - let data = "abcde".as_bytes().as_ptr(); - let res = unsafe { libc_utils::write_all(fds[1], data as *const libc::c_void, 5) }; - assert_eq!(res, 5); + // Write a bunch of data bytes to fds[1]. + let data = [42u8; 40000]; + let res = + unsafe { libc_utils::write_all(fds[1], data.as_ptr() as *const libc::c_void, data.len()) }; + assert_eq!(res, data.len() as isize); - //Two notification should be received. + // Two notification should be received. let expected_event0 = u32::try_from(libc::EPOLLIN | libc::EPOLLOUT).unwrap(); let expected_value0 = fds[0] as u64; let expected_event1 = u32::try_from(libc::EPOLLOUT).unwrap(); @@ -462,36 +456,26 @@ fn test_socketpair_read() { &[(expected_event0, expected_value0), (expected_event1, expected_value1)], ); - // Read 3 bytes from fds[0]. - let mut buf: [u8; 3] = [0; 3]; + // Read a lof of databytes from fds[0]. + // If we make this read too small, Linux won't actually trigger a notification. + // So to ensure the test works on real systems, we make a sizable read. + let mut buf = [0; 38000]; let res = unsafe { libc_utils::read_all(fds[0], buf.as_mut_ptr().cast(), buf.len() as libc::size_t) }; - assert_eq!(res, 3); - assert_eq!(buf, "abc".as_bytes()); + assert_eq!(res, buf.len() as isize); - // Notification will be provided in Miri. - // But in real systems, no notification will be provided here, since Linux prefers to avoid - // wakeups that are likely to lead to only small amounts of data being read/written. - // We make the test work in both cases, thus documenting the difference in behavior. + // fds[1] can be written now. let expected_event = u32::try_from(libc::EPOLLOUT).unwrap(); let expected_value = fds[1] as u64; - if cfg!(miri) { - check_epoll_wait::<8>(epfd, &[(expected_event, expected_value)]); - } else { - check_epoll_wait::<8>(epfd, &[]); - } + check_epoll_wait::<8>(epfd, &[(expected_event, expected_value)]); // Read until the buffer is empty. - let mut buf: [u8; 2] = [0; 2]; + let rest = data.len() - buf.len(); let res = - unsafe { libc_utils::read_all(fds[0], buf.as_mut_ptr().cast(), buf.len() as libc::size_t) }; - assert_eq!(res, 2); - assert_eq!(buf, "de".as_bytes()); + unsafe { libc_utils::read_all(fds[0], buf.as_mut_ptr().cast(), rest as libc::size_t) }; + assert_eq!(res, rest as isize); - // Notification will be provided. - // In real system, notification will be provided too. - let expected_event = u32::try_from(libc::EPOLLOUT).unwrap(); - let expected_value = fds[1] as u64; + // Again we get a notification that fds[1] can be written. check_epoll_wait::<8>(epfd, &[(expected_event, expected_value)]); } @@ -511,7 +495,7 @@ fn test_no_notification_for_unregister_flag() { events: (libc::EPOLLOUT | libc::EPOLLET) as _, u64: u64::try_from(fds[0]).unwrap(), }; - let res = unsafe { libc::epoll_ctl(epfd, libc::EPOLL_CTL_ADD, fds[1], &mut ev) }; + let res = unsafe { libc::epoll_ctl(epfd, libc::EPOLL_CTL_ADD, fds[0], &mut ev) }; assert_eq!(res, 0); // Write to fd[1]. @@ -598,7 +582,7 @@ fn test_epoll_lost_events() { let res = unsafe { libc::epoll_ctl(epfd, libc::EPOLL_CTL_ADD, fds[1], &mut ev) }; assert_eq!(res, 0); - //Two notification should be received. But we only provide buffer for one event. + // Two notification should be received. But we only provide buffer for one event. let expected_event0 = u32::try_from(libc::EPOLLOUT).unwrap(); let expected_value0 = fds[0] as u64; check_epoll_wait::<1>(epfd, &[(expected_event0, expected_value0)]); @@ -640,7 +624,8 @@ fn test_ready_list_fetching_logic() { check_epoll_wait::<1>(epfd, &[(expected_event1, expected_value1)]); } -// In epoll_ctl, if the value of epfd equals to fd, EINVAL should be returned. +// In epoll_ctl, if the value of epfd equals to fd, EFAULT should be returned. +// (The docs say loops cause EINVAL, but experiments show it is EFAULT.) fn test_epoll_ctl_epfd_equal_fd() { // Create an epoll instance. let epfd = unsafe { libc::epoll_create1(0) }; @@ -649,7 +634,7 @@ fn test_epoll_ctl_epfd_equal_fd() { let array_ptr = std::ptr::without_provenance_mut::(0x100); let res = unsafe { libc::epoll_ctl(epfd, libc::EPOLL_CTL_ADD, epfd, array_ptr) }; let e = std::io::Error::last_os_error(); - assert_eq!(e.raw_os_error(), Some(libc::EINVAL)); + assert_eq!(e.raw_os_error(), Some(libc::EFAULT)); assert_eq!(res, -1); }