Skip to content

Commit 61c5206

Browse files
committed
Provide notification everytime read is successful
1 parent 4812fa6 commit 61c5206

File tree

2 files changed

+21
-11
lines changed

2 files changed

+21
-11
lines changed

src/shims/unix/socket.rs

Lines changed: 12 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -141,14 +141,19 @@ impl FileDescription for SocketPair {
141141
// Conveniently, `read` exists on `VecDeque` and has exactly the desired behavior.
142142
let actual_read_size = readbuf.buf.read(bytes).unwrap();
143143

144-
// Notification should be provided for peer fd when read emptied the buffer.
144+
// The readbuf needs to be explicitly dropped because it will cause panic when
145+
// check_and_update_readiness borrows it again.
146+
drop(readbuf);
147+
148+
// A notification should be provided for the peer file description even when it can
149+
// only write 1 byte. This implementation is not compliant with the actual Linux kernel
150+
// implementation. For optimization reasons, the kernel will only mark the file description
151+
// as "writable" when it can write more than a certain number of bytes. Since we
152+
// don't know what that *certain number* is, we will provide a notification every time
153+
// a read is successful. This might result in our epoll emulation providing more
154+
// notifications than the real system.
145155
if let Some(peer_fd) = self.peer_fd.upgrade() {
146-
if readbuf.buf.is_empty() {
147-
// The readbuf needs to be explicitly dropped because it will cause panic when
148-
// check_and_update_readiness borrows it again.
149-
drop(readbuf);
150-
peer_fd.check_and_update_readiness(ecx)?;
151-
}
156+
peer_fd.check_and_update_readiness(ecx)?;
152157
}
153158

154159
return Ok(Ok(actual_read_size));

tests/pass-dep/libc/libc-epoll.rs

Lines changed: 9 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -485,7 +485,8 @@ fn test_event_overwrite() {
485485
assert!(check_epoll_wait::<8>(epfd, vec![(expected_event, expected_value)]));
486486
}
487487

488-
// read in socketpair will only provide epoll notification if it emptied the buffer.
488+
// An epoll notification will be provided for every succesful read in a socketpair.
489+
// This behaviour differs from the real system.
489490
fn test_socketpair_read() {
490491
// Create an epoll instance.
491492
let epfd = unsafe { libc::epoll_create1(0) };
@@ -529,8 +530,11 @@ fn test_socketpair_read() {
529530
assert_eq!(res, 3);
530531
assert_eq!(buf, "abc".as_bytes());
531532

532-
// No notification should be provided.
533-
assert!(check_epoll_wait::<8>(epfd, vec![]));
533+
// Notification will be provided.
534+
// But in real system, no notification will be provided here.
535+
let expected_event = u32::try_from(libc::EPOLLOUT).unwrap();
536+
let expected_value = fds[1] as u64;
537+
assert!(check_epoll_wait::<8>(epfd, vec![(expected_event, expected_value)]));
534538

535539
// Read until the buffer is empty.
536540
let mut buf: [u8; 2] = [0; 2];
@@ -540,7 +544,8 @@ fn test_socketpair_read() {
540544
assert_eq!(res, 2);
541545
assert_eq!(buf, "de".as_bytes());
542546

543-
// Notification should be provided.
547+
// Notification will be provided.
548+
// In real system, notification will be provided too.
544549
let expected_event = u32::try_from(libc::EPOLLOUT).unwrap();
545550
let expected_value = fds[1] as u64;
546551
assert!(check_epoll_wait::<8>(epfd, vec![(expected_event, expected_value)]));

0 commit comments

Comments
 (0)