diff --git a/src/iocore/net/P_NetAccept.h b/src/iocore/net/P_NetAccept.h index 163d5578d52..89650e34fa6 100644 --- a/src/iocore/net/P_NetAccept.h +++ b/src/iocore/net/P_NetAccept.h @@ -43,6 +43,7 @@ #include "iocore/net/NetAcceptEventIO.h" #include "Server.h" +#include #include struct NetAccept; @@ -60,21 +61,30 @@ AcceptFunction net_accept; class UnixNetVConnection; -// TODO fix race between cancel accept and call back struct NetAcceptAction : public Action, public RefCountObjInHeap { - Server *server; + std::atomic server{nullptr}; - void - cancel(Continuation *cont = nullptr) override + NetAcceptAction(Continuation *cont, Server *s) { - Action::cancel(cont); - server->close(); + continuation = cont; + if (cont != nullptr) { + mutex = cont->mutex; + } + server.store(s, std::memory_order_release); } - Continuation * - operator=(Continuation *acont) + void + cancel(Continuation *cont = nullptr) override { - return Action::operator=(acont); + // 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); + if (s != nullptr) { + s->close(); + } + if (!cancelled) { + Action::cancel(cont); + } } ~NetAcceptAction() override diff --git a/src/iocore/net/QUICNetProcessor.cc b/src/iocore/net/QUICNetProcessor.cc index 55592ef6f36..d7f7012606a 100644 --- a/src/iocore/net/QUICNetProcessor.cc +++ b/src/iocore/net/QUICNetProcessor.cc @@ -251,9 +251,7 @@ QUICNetProcessor::main_accept(Continuation *cont, SOCKET fd, AcceptOptions const na->server.sock = UnixSocket{fd}; ats_ip_copy(&na->server.accept_addr, &accept_ip); - na->action_ = new NetAcceptAction(); - *na->action_ = cont; - na->action_->server = &na->server; + na->action_ = new NetAcceptAction(cont, &na->server); na->init_accept(); return na->action_.get(); diff --git a/src/iocore/net/UnixNetAccept.cc b/src/iocore/net/UnixNetAccept.cc index cea9df78792..66e73096fa2 100644 --- a/src/iocore/net/UnixNetAccept.cc +++ b/src/iocore/net/UnixNetAccept.cc @@ -317,9 +317,7 @@ NetAccept::init_accept_per_thread() void NetAccept::stop_accept() { - if (!action_->cancelled) { - action_->cancel(); - } + action_->cancel(); server.close(); } @@ -479,6 +477,7 @@ NetAccept::acceptEvent(int event, void *ep) MUTEX_TRY_LOCK(lock, m, e->ethread); if (lock.is_locked()) { if (action_->cancelled) { + // Server was already closed by whoever called cancel(). e->cancel(); Metrics::Gauge::decrement(net_rsb.accepts_currently_open); delete this; @@ -487,6 +486,7 @@ NetAccept::acceptEvent(int event, void *ep) int res; if ((res = net_accept(this, e, false)) < 0) { + action_->cancel(); Metrics::Gauge::decrement(net_rsb.accepts_currently_open); /* INKqa11179 */ Warning("Accept on port %d failed with error no %d", ats_ip_port_host_order(&server.addr), res); @@ -637,6 +637,7 @@ NetAccept::acceptFastEvent(int event, void *ep) return EVENT_CONT; Lerror: + action_->cancel(); server.close(); e->cancel(); Metrics::Gauge::decrement(net_rsb.accepts_currently_open); @@ -656,6 +657,7 @@ NetAccept::acceptLoopEvent(int event, Event *e) } // Don't think this ever happens ... + action_->cancel(); Metrics::Gauge::decrement(net_rsb.accepts_currently_open); delete this; return EVENT_DONE; diff --git a/src/iocore/net/UnixNetProcessor.cc b/src/iocore/net/UnixNetProcessor.cc index 15630281c5c..5c8d5111e1b 100644 --- a/src/iocore/net/UnixNetProcessor.cc +++ b/src/iocore/net/UnixNetProcessor.cc @@ -133,9 +133,7 @@ UnixNetProcessor::accept_internal(Continuation *cont, int fd, AcceptOptions cons na->proxyPort = sa ? sa->proxyPort : nullptr; na->snpa = dynamic_cast(cont); - na->action_ = new NetAcceptAction(); - *na->action_ = cont; - na->action_->server = &na->server; + na->action_ = new NetAcceptAction(cont, &na->server); if (opt.frequent_accept) { // true if (accept_threads > 0 && listen_per_thread == 0) {