From 92ebdbf169fe9219bacf53f41d945d7ebf863262 Mon Sep 17 00:00:00 2001 From: Jake Smith Date: Fri, 18 Oct 2024 14:49:30 +0100 Subject: [PATCH] HPCC-32833 Reuse peerEP from accept when creating socket Signed-off-by: Jake Smith --- roxie/udplib/udpsha.hpp | 2 +- system/jlib/jsocket.cpp | 36 ++++++++++++------- system/jlib/jsocket.hpp | 2 +- system/mp/mpcomm.cpp | 18 +++++----- system/security/securesocket/securesocket.cpp | 2 +- 5 files changed, 35 insertions(+), 25 deletions(-) diff --git a/roxie/udplib/udpsha.hpp b/roxie/udplib/udpsha.hpp index 116f7e66a02..9134a80421c 100644 --- a/roxie/udplib/udpsha.hpp +++ b/roxie/udplib/udpsha.hpp @@ -347,7 +347,7 @@ class CSocketSimulator : public CInterfaceOf virtual size32_t writetms(void const* buf, size32_t minSize, size32_t size, unsigned timeoutms=WAIT_FOREVER) override { UNIMPLEMENTED; } virtual size32_t get_max_send_size() override { UNIMPLEMENTED; } - virtual ISocket* accept(bool allowcancel=false, SocketEndpoint *peerEp = nullptr) override { UNIMPLEMENTED; } + virtual ISocket* accept(bool allowcancel=false) override { UNIMPLEMENTED; } virtual int logPollError(unsigned revents, const char *rwstr) override { UNIMPLEMENTED; } virtual int wait_read(unsigned timeout) override { UNIMPLEMENTED; } virtual int wait_write(unsigned timeout) override { UNIMPLEMENTED; } diff --git a/system/jlib/jsocket.cpp b/system/jlib/jsocket.cpp index 5dfd4acde52..851f2e0530c 100644 --- a/system/jlib/jsocket.cpp +++ b/system/jlib/jsocket.cpp @@ -654,7 +654,7 @@ class CSocket: public ISocket, public CInterface void shutdown(unsigned mode=SHUTDOWN_READWRITE); void shutdownNoThrow(unsigned mode); - ISocket* accept(bool allowcancel, SocketEndpoint *peerEp=nullptr); + ISocket* accept(bool allowcancel); int wait_read(unsigned timeout); int logPollError(unsigned revents, const char *rwstr); int wait_write(unsigned timeout); @@ -703,7 +703,7 @@ class CSocket: public ISocket, public CInterface void setTraceName(); CSocket(const SocketEndpoint &_ep,SOCKETMODE smode,const char *name); - CSocket(T_SOCKET new_sock,SOCKETMODE smode,bool _owned); + CSocket(T_SOCKET new_sock,SOCKETMODE smode,bool _owned,SocketEndpoint *_peerEp); virtual ~CSocket(); @@ -1254,7 +1254,7 @@ void CSocket::open(int listen_queue_size,bool reuseports) -ISocket* CSocket::accept(bool allowcancel, SocketEndpoint *peerEp) +ISocket* CSocket::accept(bool allowcancel) { if ((accept_cancel_state!=accept_not_cancelled) && allowcancel) { accept_cancel_state=accept_cancelled; @@ -1334,10 +1334,9 @@ ISocket* CSocket::accept(bool allowcancel, SocketEndpoint *peerEp) THROWJSOCKTARGETEXCEPTION(JSOCKERR_cancel_accept); } - if (peerEp) - getSockAddrEndpoint(peerSockAddr, peerSockAddrLen, *peerEp); - - CSocket *ret = new CSocket(newsock,sm_tcp,true); + SocketEndpoint peerEp; + getSockAddrEndpoint(peerSockAddr, peerSockAddrLen, peerEp); + CSocket *ret = new CSocket(newsock,sm_tcp,true,&peerEp); ret->checkCfgKeepAlive(); ret->set_inherit(false); ret->set_nonblock(true); @@ -3023,7 +3022,7 @@ CSocket::CSocket(const SocketEndpoint &ep,SOCKETMODE smode,const char *name) #endif } -CSocket::CSocket(T_SOCKET new_sock,SOCKETMODE smode,bool _owned) +CSocket::CSocket(T_SOCKET new_sock,SOCKETMODE smode,bool _owned,SocketEndpoint *_peerEp) { nonblocking = false; #ifdef USERECVSEM @@ -3045,13 +3044,24 @@ CSocket::CSocket(T_SOCKET new_sock,SOCKETMODE smode,bool _owned) accept_cancel_state = accept_not_cancelled; set_nagle(false); //set_linger(DEFAULT_LINGER_TIME); -- experiment with removing this as closesocket should still endevour to send outstanding data - char peer[256]; - hostport = peer_name(peer,sizeof(peer)); - targetip.ipset(peer); + if (_peerEp) + { + targetip = *_peerEp; + hostport = _peerEp->port; + } + else + { + char peer[256]; + hostport = peer_name(peer,sizeof(peer)); + targetip.ipset(peer); + } SocketEndpoint ep; localPort = getEndpoint(ep).port; #ifdef _TRACE - setTraceName("A!", peer); + StringBuffer tmp; + targetip.getIpText(tmp); + tmp.append(":").append(hostport); + setTraceName("A!", tmp); #endif } @@ -3148,7 +3158,7 @@ ISocket* ISocket::multicast_connect(const SocketEndpoint &ep, unsigned _ttl) ISocket* ISocket::attach(int s, bool tcpip) { - CSocket* sock = new CSocket((SOCKET)s, tcpip?sm_tcp:sm_udp, false); + CSocket* sock = new CSocket((SOCKET)s, tcpip?sm_tcp:sm_udp, false, nullptr); sock->set_nonblock(true); return sock; } diff --git a/system/jlib/jsocket.hpp b/system/jlib/jsocket.hpp index 08524d55b72..fc46866efeb 100644 --- a/system/jlib/jsocket.hpp +++ b/system/jlib/jsocket.hpp @@ -308,7 +308,7 @@ class jlib_decl ISocket : extends IInterface // // This method is called by server to accept client connection // - virtual ISocket* accept(bool allowcancel=false, SocketEndpoint *peerEp = nullptr) = 0; // not needed for UDP + virtual ISocket* accept(bool allowcancel=false) = 0; // not needed for UDP // // log poll() errors diff --git a/system/mp/mpcomm.cpp b/system/mp/mpcomm.cpp index aa69fc8cfa4..3e2495a1cf6 100644 --- a/system/mp/mpcomm.cpp +++ b/system/mp/mpcomm.cpp @@ -221,7 +221,6 @@ struct MultiPacketHeader class DECL_EXCEPTION CMPException: public IMP_Exception, public CInterface { - StringAttr msg; public: IMPLEMENT_IINTERFACE; @@ -254,6 +253,7 @@ class DECL_EXCEPTION CMPException: public IMP_Exception, public CInterface private: MessagePassingError error; SocketEndpoint endpoint; + StringAttr msg; }; @@ -537,7 +537,6 @@ class CMPConnectThread: public Thread { CConnectSelectHandler &selectHandler; Owned sock; - SocketEndpoint peerEP; StringBuffer peerHostText, peerEndpointText; ConnectHdr hdr; cycle_t createTime = 0; @@ -545,11 +544,13 @@ class CMPConnectThread: public Thread CriticalSection crit; bool closedOrHandled = false; public: - CSocketHandler(CConnectSelectHandler &_selectHandler, ISocket *_sock, const SocketEndpoint &_peerEP) : selectHandler(_selectHandler), sock(_sock), peerEP(_peerEP) + CSocketHandler(CConnectSelectHandler &_selectHandler, ISocket *_sock) : selectHandler(_selectHandler), sock(_sock) { createTime = get_cycles_now(); + SocketEndpoint peerEP; + sock->getPeerEndpoint(peerEP); peerEP.getHostText(peerHostText); // always used by handleAcceptedSocket - peerEndpointText.append(peerEndpointText); // only used if tracing an error + peerEndpointText.append(peerHostText); // only used if tracing an error if (peerEP.port) peerEndpointText.append(':').append(peerEP.port); } @@ -692,7 +693,7 @@ class CMPConnectThread: public Thread maintenanceSem.signal(); maintenanceThread.join(); } - void add(ISocket *sock, const SocketEndpoint &peerEP) + void add(ISocket *sock) { while (true) { @@ -707,7 +708,7 @@ class CMPConnectThread: public Thread MilliSleep(1000); } - Owned socketHandler = new CSocketHandler(*this, LINK(sock), peerEP); + Owned socketHandler = new CSocketHandler(*this, LINK(sock)); size_t numHandlers; { @@ -2569,10 +2570,9 @@ int CMPConnectThread::run() while (running) { Owned sock; - SocketEndpoint peerEP; try { - sock.setown(listensock->accept(true, &peerEP)); + sock.setown(listensock->accept(true)); } catch (IException *e) { @@ -2611,7 +2611,7 @@ int CMPConnectThread::run() // After that, the socket will be removed from the connectSelectHamndler, // a CMPChannel will be estalbished, and the socket will be added to the MP CMPPacketReader select handler. // See handleAcceptedSocket. - connectSelectHandler.add(sock, peerEP); + connectSelectHandler.add(sock); } else { diff --git a/system/security/securesocket/securesocket.cpp b/system/security/securesocket/securesocket.cpp index f8b8c9a4f7c..623c6ea52e2 100644 --- a/system/security/securesocket/securesocket.cpp +++ b/system/security/securesocket/securesocket.cpp @@ -221,7 +221,7 @@ class CSecureSocket : implements ISecureSocket, public CInterface // // This method is called by server to accept client connection // - virtual ISocket* accept(bool allowcancel=false, SocketEndpoint *peerEp=nullptr) // not needed for UDP + virtual ISocket* accept(bool allowcancel=false) // not needed for UDP { throw MakeStringException(-1, "CSecureSocket::accept: not implemented"); }