Skip to content

Commit

Permalink
Don't watch for EPOLLRDHUP in epoll
Browse files Browse the repository at this point in the history
Currently, EPOLLRDHUP sets UnixReady::hup(). This is incorrect behavior
because higher-level libraries like tokio (correctly) assume that
UnixReady::hup() is unclearable since it signals that both the read and
write halfs are shutdown. However, EPOLLRDHUP only means that the TCP
stream has been half-closed and, in reality, such a half-closed stream
can still be written to.

This will fix a current issue with tokio, which is that tokio infinitely
attempts to write to a half-closed socket that's returning WouldBlock
when it's write buffer is full, an issue which manifests with excessive
CPU usage. I think this may help some of the issues discussed in
tokio-rs/tokio#449

After this change, EOF will still be propagated correctly, because
read-hangups also trigger read-readiness via EPOLLIN. However, if
handling of EPOLLRDHUP is desired to be retained, I believe it should be
implemented as another readiness kind on UnixReady, perhaps
UnixReady::read_hup().

Possible concern of a breaking change: Since it's not currently possible
for a user of mio to differentiate between EPOLLHUP and EPOLLRDHUP, it
must be that no users of mio currently are. There _may_ be applications
that test the "health" of a socket by checking for UnixRead::hup(),
which would previously trigger on EPOLLRDHUP but will no longer with
this change. This will change such applications from considering a
half-closed connection as closed to considering it open. However, I
still beleive this change is a correction of the semantics of HUP and
the desired behavior such applications was already ambiguous.

Note: if this is an agreed upon change for epoll, I think a similar
change is in order for kqueue. I _think_ this would be to only set
UnixReady::hup() if (e.filter == libc::EVFILT_READ && e.flags &
libc::EV_EOF != 0), but I will leave that change to someone more
knowledgeable with kqueue.
  • Loading branch information
behrat committed May 1, 2019
1 parent c24bba7 commit 957e96d
Showing 1 changed file with 2 additions and 6 deletions.
8 changes: 2 additions & 6 deletions src/sys/unix/epoll.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ use std::time::Duration;
use std::{cmp, i32};

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

use event_imp::Event;
Expand Down Expand Up @@ -166,10 +166,6 @@ fn ioevent_to_epoll(interest: Ready, opts: PollOpt) -> u32 {
kind |= EPOLLOUT;
}

if UnixReady::from(interest).is_hup() {
kind |= EPOLLRDHUP;
}

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

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

Expand Down

0 comments on commit 957e96d

Please sign in to comment.