TS-4796 Change UnixNetHandler to always bubble up epoll errors to the 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.

Author: jacksontj

iocore/net/P_UnixNetState.h

@@ -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

iocore/net/P_UnixNetVConnection.h

@@ -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);

iocore/net/UnixNet.cc

@@ -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) {
@@ -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) {
@@ -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);
@@ -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);
@@ -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);
@@ -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);

iocore/net/UnixNetVConnection.cc

@@ -260,6 +260,19 @@ 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);
@@ -427,6 +440,17 @@ 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()) {
@@ -1079,6 +1103,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)
 {