From 883e4773b30f652e83326df99dc0ec7d652e1f94 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Fri, 16 Aug 2024 17:06:37 +0200 Subject: [PATCH] explain the behavior on closed peers --- src/tools/miri/src/shims/unix/linux/epoll.rs | 12 +++++++----- src/tools/miri/src/shims/unix/socket.rs | 10 ++++------ src/tools/miri/tests/pass-dep/libc/libc-epoll.rs | 1 + 3 files changed, 12 insertions(+), 11 deletions(-) diff --git a/src/tools/miri/src/shims/unix/linux/epoll.rs b/src/tools/miri/src/shims/unix/linux/epoll.rs index ce91712fb27..d529a9b63e6 100644 --- a/src/tools/miri/src/shims/unix/linux/epoll.rs +++ b/src/tools/miri/src/shims/unix/linux/epoll.rs @@ -433,11 +433,13 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> { Ok(Scalar::from_i32(num_of_events)) } - /// For a specific file description, get its ready events and update - /// the corresponding ready list. This function is called whenever a file description - /// is registered with epoll, or the buffer it reads from / writes to changed. - /// 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! + /// 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. + /// + /// 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(&self, fd_ref: &FileDescriptionRef) -> InterpResult<'tcx, ()> { let this = self.eval_context_ref(); let id = fd_ref.get_id(); diff --git a/src/tools/miri/src/shims/unix/socket.rs b/src/tools/miri/src/shims/unix/socket.rs index cdc9dd0429b..75d4d0dbbe4 100644 --- a/src/tools/miri/src/shims/unix/socket.rs +++ b/src/tools/miri/src/shims/unix/socket.rs @@ -71,9 +71,9 @@ impl FileDescription for SocketPair { } else { // Peer FD has been closed. epoll_ready_events.epollrdhup = true; - // This is an edge case. Whenever epollrdhup is triggered, epollin and epollout will be - // added even though there is no data in the buffer. - // FIXME: Figure out why. This looks like a bug. + // Since the peer is closed, even if no data is available reads will return EOF and + // writes will return EPIPE. In other words, they won't block, so we mark this as ready + // for read and write. epoll_ready_events.epollin = true; epoll_ready_events.epollout = true; } @@ -86,9 +86,7 @@ impl FileDescription for SocketPair { ecx: &mut MiriInterpCx<'tcx>, ) -> InterpResult<'tcx, io::Result<()>> { if let Some(peer_fd) = self.peer_fd().upgrade() { - // Notify peer fd that closed has happened. - // When any of the events happened, we check and update the status of all supported events - // types of peer fd. + // Notify peer fd that close has happened, since that can unblock reads and writes. ecx.check_and_update_readiness(&peer_fd)?; } Ok(Ok(())) diff --git a/src/tools/miri/tests/pass-dep/libc/libc-epoll.rs b/src/tools/miri/tests/pass-dep/libc/libc-epoll.rs index f8bf1bacff3..841761e53dd 100644 --- a/src/tools/miri/tests/pass-dep/libc/libc-epoll.rs +++ b/src/tools/miri/tests/pass-dep/libc/libc-epoll.rs @@ -96,6 +96,7 @@ fn test_epoll_socketpair() { assert_eq!(res, 0); // 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).unwrap(); let expected_value = u64::try_from(fds[1]).unwrap(); assert!(check_epoll_wait::<8>(epfd, &[(expected_event, expected_value)]));