From f38db639645fec5442d10fed79589c1159ca9931 Mon Sep 17 00:00:00 2001 From: Eduard Bagdasaryan Date: Thu, 14 Jul 2022 21:33:40 +0000 Subject: [PATCH] Preserve caller context in TcpAcceptor (#1087) Before this fix, acceptOne() was losing FTP DATA TcpAcceptor context by unconditionally resetting the context to a nil listenPort_. Also, TcpAcceptor methods should not (be expected to) explicitly set their job code context. Instead, the job should be created in the right code context, allowing job-created async calls to auto-manage code context. The updated port-iterating clientHttpConnectionsOpen() and Ftp::StartListening() loops now set the right code context when creating async calls that result in TcpAcceptor job creation. --- src/client_side.cc | 6 ++++++ src/comm/TcpAcceptor.cc | 36 +++++++++++++++++------------------- src/servers/FtpServer.cc | 6 ++++++ 3 files changed, 29 insertions(+), 19 deletions(-) diff --git a/src/client_side.cc b/src/client_side.cc index 01d42f4a81..3dcb8eb106 100644 --- a/src/client_side.cc +++ b/src/client_side.cc @@ -3303,7 +3303,9 @@ AddOpenedHttpSocket(const Comm::ConnectionPointer &conn) static void clientHttpConnectionsOpen(void) { + const auto savedContext = CodeContext::Current(); for (AnyP::PortCfgPointer s = HttpPortList; s != nullptr; s = s->next) { + CodeContext::Reset(s); const SBuf &scheme = AnyP::UriScheme(s->transport.protocol).image(); if (MAXTCPLISTENPORTS == NHttpSockets) { @@ -3344,6 +3346,7 @@ clientHttpConnectionsOpen(void) CommAcceptCbPtrFun(isHttps ? httpsAccept : httpAccept, CommAcceptCbParams(nullptr))); clientStartListeningOn(s, subCall, isHttps ? Ipc::fdnHttpsSocket : Ipc::fdnHttpSocket); } + CodeContext::Reset(savedContext); } void @@ -3424,13 +3427,16 @@ clientOpenListenSockets(void) void clientConnectionsClose() { + const auto savedContext = CodeContext::Current(); for (AnyP::PortCfgPointer s = HttpPortList; s != nullptr; s = s->next) { + CodeContext::Reset(s); if (s->listenConn != nullptr) { debugs(1, Important(14), "Closing HTTP(S) port " << s->listenConn->local); s->listenConn->close(); s->listenConn = nullptr; } } + CodeContext::Reset(savedContext); Ftp::StopListening(); diff --git a/src/comm/TcpAcceptor.cc b/src/comm/TcpAcceptor.cc index ab45b941a2..42f73c5397 100644 --- a/src/comm/TcpAcceptor.cc +++ b/src/comm/TcpAcceptor.cc @@ -11,6 +11,7 @@ #include "squid.h" #include "acl/FilledChecklist.h" #include "anyp/PortCfg.h" +#include "base/CodeContext.h" #include "base/TextException.h" #include "client_db.h" #include "comm/AcceptLimiter.h" @@ -72,8 +73,6 @@ Comm::TcpAcceptor::unsubscribe(const char *reason) void Comm::TcpAcceptor::start() { - if (listenPort_) - CodeContext::Reset(listenPort_); debugs(5, 5, status() << " AsyncCall Subscription: " << theCallSub); Must(IsConnOpen(conn)); @@ -249,17 +248,16 @@ void Comm::TcpAcceptor::logAcceptError(const ConnectionPointer &tcpClient) const { AccessLogEntry::Pointer al = new AccessLogEntry; - CodeContext::Reset(al); - al->tcpClient = tcpClient; - al->url = "error:accept-client-connection"; - al->setVirginUrlForMissingRequest(al->url); - ACLFilledChecklist ch(nullptr, nullptr, nullptr); - ch.src_addr = tcpClient->remote; - ch.my_addr = tcpClient->local; - ch.al = al; - accessLogLog(al, &ch); - - CodeContext::Reset(listenPort_); + CallBack(al, [&] { + al->tcpClient = tcpClient; + al->url = "error:accept-client-connection"; + al->setVirginUrlForMissingRequest(al->url); + ACLFilledChecklist ch(nullptr, nullptr, nullptr); + ch.src_addr = tcpClient->remote; + ch.my_addr = tcpClient->local; + ch.al = al; + accessLogLog(al, &ch); + }); } void @@ -291,12 +289,12 @@ Comm::TcpAcceptor::acceptOne() debugs(5, 5, "try later: " << conn << " handler Subscription: " << theCallSub); } else { // TODO: When ALE, MasterXaction merge, use them or ClientConn instead. - CodeContext::Reset(newConnDetails); - debugs(5, 5, "Listener: " << conn << - " accepted new connection " << newConnDetails << - " handler Subscription: " << theCallSub); - notify(flag, newConnDetails); - CodeContext::Reset(listenPort_); + CallBack(newConnDetails, [&] { + debugs(5, 5, "Listener: " << conn << + " accepted new connection " << newConnDetails << + " handler Subscription: " << theCallSub); + notify(flag, newConnDetails); + }); } SetSelect(conn->fd, COMM_SELECT_READ, doAccept, this, 0); diff --git a/src/servers/FtpServer.cc b/src/servers/FtpServer.cc index 7c9b93d1b4..1eb2603bc4 100644 --- a/src/servers/FtpServer.cc +++ b/src/servers/FtpServer.cc @@ -264,7 +264,9 @@ Ftp::Server::AcceptCtrlConnection(const CommAcceptCbParams ¶ms) void Ftp::StartListening() { + const auto savedContext = CodeContext::Current(); for (AnyP::PortCfgPointer s = FtpPortList; s != nullptr; s = s->next) { + CodeContext::Reset(s); if (MAXTCPLISTENPORTS == NHttpSockets) { debugs(1, DBG_IMPORTANT, "Ignoring ftp_port lines exceeding the" << " limit of " << MAXTCPLISTENPORTS << " ports."); @@ -278,18 +280,22 @@ Ftp::StartListening() CommAcceptCbParams(nullptr))); clientStartListeningOn(s, subCall, Ipc::fdnFtpSocket); } + CodeContext::Reset(savedContext); } void Ftp::StopListening() { + const auto savedContext = CodeContext::Current(); for (AnyP::PortCfgPointer s = FtpPortList; s != nullptr; s = s->next) { + CodeContext::Reset(s); if (s->listenConn != nullptr) { debugs(1, DBG_IMPORTANT, "Closing FTP port " << s->listenConn->local); s->listenConn->close(); s->listenConn = nullptr; } } + CodeContext::Reset(savedContext); } void -- 2.47.2