Skip to content

Commit

Permalink
TS-4796 Change UnixNetHandler to always bubble up epoll errors to the…
Browse files Browse the repository at this point in the history
… VConnection

Before if the vcon wasn't read or write enabled errors would be swallowed. This leads to a variety of issues where the socket errors aren't dealt with immediately.
  • Loading branch information
jacksontj committed Oct 26, 2016
1 parent c2dd884 commit c1ac5f8
Show file tree
Hide file tree
Showing 4 changed files with 51 additions and 7 deletions.
3 changes: 2 additions & 1 deletion iocore/net/P_UnixNetState.h
Original file line number Diff line number Diff line change
Expand Up @@ -48,13 +48,14 @@ class UnixNetVConnection;

struct NetState {
volatile int enabled;
volatile int error;
VIO vio;
Link<UnixNetVConnection> ready_link;
SLink<UnixNetVConnection> enable_link;
int in_enabled_list;
int triggered;

NetState() : enabled(0), vio(VIO::NONE), in_enabled_list(0), triggered(0) {}
NetState() : enabled(0), error(0), vio(VIO::NONE), in_enabled_list(0), triggered(0) {}
};

#endif
1 change: 1 addition & 0 deletions iocore/net/P_UnixNetVConnection.h
Original file line number Diff line number Diff line change
Expand Up @@ -234,6 +234,7 @@ class UnixNetVConnection : public NetVConnection
virtual int64_t load_buffer_and_write(int64_t towrite, MIOBufferAccessor &buf, int64_t &total_written, int &needs);
void readDisable(NetHandler *nh);
void readSignalError(NetHandler *nh, int err);
void writeSignalError(NetHandler *nh, int err);
int readSignalDone(int event, NetHandler *nh);
int readSignalAndUpdate(int event);
void readReschedule(NetHandler *nh);
Expand Down
22 changes: 16 additions & 6 deletions iocore/net/UnixNet.cc
Original file line number Diff line number Diff line change
Expand Up @@ -450,8 +450,13 @@ NetHandler::mainNetEvent(int event, Event *e)
if (cop_list.in(vc)) {
cop_list.remove(vc);
}
if (get_ev_events(pd, x) & (EVENTIO_READ | EVENTIO_ERROR)) {
if (get_ev_events(pd, x) & EVENTIO_READ) {
vc->read.triggered = 1;
if (get_ev_events(pd, x) & EVENTIO_ERROR) {
vc->read.error = 1;
} else {
vc->read.error = 0;
}
if (!read_ready_list.in(vc)) {
read_ready_list.enqueue(vc);
} else if (get_ev_events(pd, x) & EVENTIO_ERROR) {
Expand All @@ -461,8 +466,13 @@ NetHandler::mainNetEvent(int event, Event *e)
}
}
vc = epd->data.vc;
if (get_ev_events(pd, x) & (EVENTIO_WRITE | EVENTIO_ERROR)) {
if (get_ev_events(pd, x) & EVENTIO_WRITE) {
vc->write.triggered = 1;
if (get_ev_events(pd, x) & EVENTIO_ERROR) {
vc->write.error = 1;
} else {
vc->write.error = 0;
}
if (!write_ready_list.in(vc)) {
write_ready_list.enqueue(vc);
} else if (get_ev_events(pd, x) & EVENTIO_ERROR) {
Expand Down Expand Up @@ -495,7 +505,7 @@ NetHandler::mainNetEvent(int event, Event *e)
set_cont_flags(vc->control_flags);
if (vc->closed)
close_UnixNetVConnection(vc, trigger_event->ethread);
else if (vc->read.enabled && vc->read.triggered)
else if ((vc->read.enabled || vc->read.error) && vc->read.triggered)
vc->net_read_io(this, trigger_event->ethread);
else if (!vc->read.enabled) {
read_ready_list.remove(vc);
Expand All @@ -512,7 +522,7 @@ NetHandler::mainNetEvent(int event, Event *e)
set_cont_flags(vc->control_flags);
if (vc->closed)
close_UnixNetVConnection(vc, trigger_event->ethread);
else if (vc->write.enabled && vc->write.triggered)
else if ((vc->write.enabled || vc->write.error) && vc->write.triggered)
write_to_net(this, vc, trigger_event->ethread);
else if (!vc->write.enabled) {
write_ready_list.remove(vc);
Expand All @@ -530,7 +540,7 @@ NetHandler::mainNetEvent(int event, Event *e)
diags->set_override(vc->control.debug_override);
if (vc->closed)
close_UnixNetVConnection(vc, trigger_event->ethread);
else if (vc->read.enabled && vc->read.triggered)
else if ((vc->read.enabled || vc->read.error) && vc->read.triggered)
vc->net_read_io(this, trigger_event->ethread);
else if (!vc->read.enabled)
vc->ep.modify(-EVENTIO_READ);
Expand All @@ -539,7 +549,7 @@ NetHandler::mainNetEvent(int event, Event *e)
diags->set_override(vc->control.debug_override);
if (vc->closed)
close_UnixNetVConnection(vc, trigger_event->ethread);
else if (vc->write.enabled && vc->write.triggered)
else if ((vc->write.enabled || vc->write.error) && vc->write.triggered)
write_to_net(this, vc, trigger_event->ethread);
else if (!vc->write.enabled)
vc->ep.modify(-EVENTIO_WRITE);
Expand Down
32 changes: 32 additions & 0 deletions iocore/net/UnixNetVConnection.cc
Original file line number Diff line number Diff line change
Expand Up @@ -260,6 +260,20 @@ read_from_net(NetHandler *nh, UnixNetVConnection *vc, EThread *thread)
close_UnixNetVConnection(vc, thread);
return;
}

if (!s->enabled && vc->read.error) {
int err = 0, errlen = sizeof(int);
if (getsockopt(vc->con.fd, SOL_SOCKET, SO_ERROR, &err, (socklen_t *)&errlen) == -1) {
err = errno;
}

// if it is a non-temporary error, we should die appropriately
if (err && err != EAGAIN && err != EINTR) {
read_signal_error(nh, vc, err);
return;
}
}

// if it is not enabled.
if (!s->enabled || s->vio.op != VIO::READ) {
read_disable(nh, vc);
Expand Down Expand Up @@ -427,6 +441,18 @@ write_to_net_io(NetHandler *nh, UnixNetVConnection *vc, EThread *thread)
return;
}

if (!s->enabled && vc->write.error) {
int err = 0, errlen = sizeof(int);
if (getsockopt(vc->con.fd, SOL_SOCKET, SO_ERROR, &err, (socklen_t *)&errlen) == -1) {
err = errno;
}

if (err && err != EAGAIN && err != EINTR) {
write_signal_error(nh, vc, err);
return;
}
}

// This function will always return true unless
// vc is an SSLNetVConnection.
if (!vc->getSSLHandShakeComplete()) {
Expand Down Expand Up @@ -1079,6 +1105,12 @@ UnixNetVConnection::readSignalError(NetHandler *nh, int err)
read_signal_error(nh, this, err);
}

void
UnixNetVConnection::writeSignalError(NetHandler *nh, int err)
{
write_signal_error(nh, this, err);
}

int
UnixNetVConnection::readSignalDone(int event, NetHandler *nh)
{
Expand Down

0 comments on commit c1ac5f8

Please sign in to comment.