Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Don't report RDHUP as HUP #939

Merged
merged 12 commits into from May 15, 2019
4 changes: 2 additions & 2 deletions azure-pipelines.yml
@@ -1,5 +1,5 @@
trigger: ["master"]
pr: ["master"]
trigger: ["master", "v0.6.x"]
pr: ["master", "v0.6.x"]

jobs:
# Check formatting
Expand Down
9 changes: 2 additions & 7 deletions src/sys/unix/epoll.rs
Expand Up @@ -6,7 +6,7 @@ use std::time::Duration;
use std::{cmp, i32};

use libc::{self, c_int};
use libc::{EPOLLERR, EPOLLHUP, EPOLLRDHUP, EPOLLONESHOT};
use libc::{EPOLLERR, EPOLLHUP, EPOLLONESHOT};
use libc::{EPOLLET, EPOLLOUT, EPOLLIN, EPOLLPRI};

use {io, Ready, PollOpt, Token};
Expand Down Expand Up @@ -141,11 +141,6 @@ fn ioevent_to_epoll(interest: Ready, opts: PollOpt) -> u32 {
kind |= EPOLLOUT;
}

if UnixReady::from(interest).is_hup() {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So this means passing HUP readiness is a no-op on epoll platforms right?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, HUP is an implicit interest w/ epoll (you always get it). The error here was requesting RDHUP which means something different.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Right EPOLLHUP is implicit but it won't be possible to get EPOLLRDHUP anymore right?

kind |= EPOLLRDHUP;
}


if UnixReady::from(interest).is_priority() {
kind |= EPOLLPRI;
}
Expand Down Expand Up @@ -228,7 +223,7 @@ impl Events {
kind = kind | UnixReady::error();
}

if (epoll & EPOLLRDHUP) != 0 || (epoll & EPOLLHUP) != 0 {
if (epoll & EPOLLHUP) != 0 {
kind = kind | UnixReady::hup();
}

Expand Down
23 changes: 13 additions & 10 deletions src/sys/unix/kqueue.rs
Expand Up @@ -291,6 +291,19 @@ impl Events {
event::kind_mut(&mut self.events[idx]).insert(Ready::readable());
} else if e.filter == libc::EVFILT_WRITE as Filter {
event::kind_mut(&mut self.events[idx]).insert(Ready::writable());

carllerche marked this conversation as resolved.
Show resolved Hide resolved

// `EV_EOF` set with `EVFILT_WRITE` indicates the connection has been fully
// disconnected (read and write), but only sockets...
if e.flags & libc::EV_EOF != 0 {
event::kind_mut(&mut self.events[idx]).insert(UnixReady::hup());

// When the read end of the socket is closed, EV_EOF is set on
// flags, and fflags contains the error if there is one.
if e.fflags != 0 {
event::kind_mut(&mut self.events[idx]).insert(UnixReady::error());
}
}
}
#[cfg(any(target_os = "dragonfly",
target_os = "freebsd", target_os = "ios", target_os = "macos"))]
Expand All @@ -305,16 +318,6 @@ impl Events {
event::kind_mut(&mut self.events[idx]).insert(UnixReady::lio());
}
}

if e.flags & libc::EV_EOF != 0 {
event::kind_mut(&mut self.events[idx]).insert(UnixReady::hup());

// When the read end of the socket is closed, EV_EOF is set on
// flags, and fflags contains the error if there is one.
if e.fflags != 0 {
event::kind_mut(&mut self.events[idx]).insert(UnixReady::error());
}
}
}

ret
Expand Down
1 change: 1 addition & 0 deletions test/mod.rs
Expand Up @@ -30,6 +30,7 @@ mod test_reregister_without_poll;
mod test_smoke;
mod test_tcp;
mod test_tcp_level;
mod test_tcp_shutdown;
mod test_udp_level;
mod test_udp_socket;
mod test_write_then_drop;
Expand Down
73 changes: 73 additions & 0 deletions test/test_tcp_shutdown.rs
@@ -0,0 +1,73 @@
use std::net::Shutdown;
use std::time::Duration;

use mio::{Token, Ready, PollOpt, Poll, Events};
use mio::net::TcpStream;

macro_rules! wait {
($poll:ident, $ready:ident) => {{
use std::time::Instant;

let now = Instant::now();
let mut events = Events::with_capacity(16);
let mut found = false;

while !found {
if now.elapsed() > Duration::from_secs(5) {
panic!("not ready");
}

$poll.poll(&mut events, Some(Duration::from_secs(1))).unwrap();

for event in &events {
#[cfg(unix)]
{
use mio::unix::UnixReady;
assert!(!UnixReady::from(event.readiness()).is_hup());
}

if event.token() == Token(0) && event.readiness().$ready() {
found = true;
break;
}
}
}
}};
}

#[test]
fn test_write_shutdown() {
let poll = Poll::new().unwrap();

let listener = std::net::TcpListener::bind("127.0.0.1:0").unwrap();
let addr = listener.local_addr().unwrap();

let mut ready = Ready::readable() | Ready::writable();

#[cfg(unix)]
{
ready |= mio::unix::UnixReady::hup();
}

let client = TcpStream::connect(&addr).unwrap();
poll.register(&client,
Token(0),
ready,
PollOpt::edge()).unwrap();
carllerche marked this conversation as resolved.
Show resolved Hide resolved

let (socket, _) = listener.accept().unwrap();

wait!(poll, is_writable);

let mut events = Events::with_capacity(16);

// Polling should not have any events
poll.poll(&mut events, Some(Duration::from_millis(100))).unwrap();
assert!(events.iter().next().is_none());

println!("SHUTTING DOWN");
// Now, shutdown the write half of the socket.
socket.shutdown(Shutdown::Write).unwrap();

wait!(poll, is_readable);
}