fix cancel/accept race in NetAccept that causes EBADF abort

src/iocore/net/P_NetAccept.h

@@ -62,23 +62,38 @@ AcceptFunction net_accept;
 class UnixNetVConnection;
 
 struct NetAcceptAction : public Action, public RefCountObjInHeap {
-  std::atomic<Server *> server{nullptr};
-
   NetAcceptAction(Continuation *cont, Server *s)
   {
     continuation = cont;
     if (cont != nullptr) {
       mutex = cont->mutex;
     }
-    server.store(s, std::memory_order_release);
+    _server.store(s, std::memory_order_release);
+  }
+
+  /** Whether this action still owns an open listening socket.
+
+    Accept paths must consult this before dispatching EVENT_ERROR. It is
+    cleared by cancel() before Action::cancel() sets @c cancelled, so it goes
+    false no later than the cancellation the continuation is aware of, and it
+    is safe to read from any thread. Reading @c cancelled instead is both a
+    data race on a plain bool and too late: accept() reports EBADF as soon as
+    the socket closes, which is before @c cancelled is set.
+
+  */
+  bool
+  is_listening() const
+  {
+    return _server.load(std::memory_order_acquire) != nullptr;
   }
 
   void
   cancel(Continuation *cont = nullptr) override
   {
     // Use atomic exchange so only one thread closes the server, preventing
-    // use-after-free races between cancel() and acceptEvent() cleanup.
-    Server *s = server.exchange(nullptr, std::memory_order_acq_rel);
+    // use-after-free races between cancel() and acceptEvent() cleanup. This
+    // must stay ahead of Action::cancel(), see is_listening().
+    Server *s = _server.exchange(nullptr, std::memory_order_acq_rel);
     if (s != nullptr) {
       s->close();
     }
@@ -92,6 +107,11 @@ struct NetAcceptAction : public Action, public RefCountObjInHeap {
     static DbgCtl dbg_ctl{"net_accept"};
     Dbg(dbg_ctl, "NetAcceptAction dying");
   }
+
+private:
+  /// Cleared exactly once, by cancel(). Private so the ordering above is the
+  /// only way this transitions to null.
+  std::atomic<Server *> _server{nullptr};
 };
 
 //

src/iocore/net/UnixNetAccept.cc

@@ -112,7 +112,7 @@ net_accept(NetAccept *na, void *ep, bool blockable)
       if (res == -EAGAIN || res == -ECONNABORTED || res == -EPIPE) {
         goto Ldone;
       }
-      if (na->server.sock.is_ok() && !na->action_->cancelled) {
+      if (na->action_->is_listening()) {
         if (!blockable) {
           na->action_->continuation->handleEvent(EVENT_ERROR, reinterpret_cast<void *>(res));
         } else {
@@ -387,7 +387,7 @@ NetAccept::do_blocking_accept(EThread *t)
       case -1:
         [[fallthrough]];
       default:
-        if (!action_->cancelled) {
+        if (action_->is_listening()) {
           SCOPED_MUTEX_LOCK(lock, action_->mutex ? action_->mutex : t->mutex, t);
           action_->continuation->handleEvent(EVENT_ERROR, reinterpret_cast<void *>(res));
           Warning("accept thread received fatal error: errno = %d", errno);
@@ -580,7 +580,7 @@ NetAccept::acceptFastEvent(int event, void *ep)
         check_transient_accept_error(res);
         goto Ldone;
       }
-      if (!action_->cancelled) {
+      if (action_->is_listening()) {
         action_->continuation->handleEvent(EVENT_ERROR, reinterpret_cast<void *>(res));
       }
       goto Lerror;