Browse Source

Merge pull request #15617 from ghalliday/issue26873

HPCC-26873 Use ISocket::shutdown when terminating read sockets

Reviewed-By: Mark Kelly <mark.kelly@lexisnexis.com>
Reviewed-By: Richard Chapman <rchapman@hpccsystems.com>
Richard Chapman 3 years ago
parent
commit
02e4d157d1

+ 1 - 2
common/thorhelper/persistent.cpp

@@ -218,8 +218,7 @@ public:
             if(m_doNotReuseList.getValue(epstr.str()) != nullptr)
             if(m_doNotReuseList.getValue(epstr.str()) != nullptr)
             {
             {
                 PERSILOG(PersistentLogLevel::PLogNormal, "PERSISTENT: socket %d's target endpoint %s is in DoNotReuseList, will not add it.", sock->OShandle(), epstr.str());
                 PERSILOG(PersistentLogLevel::PLogNormal, "PERSISTENT: socket %d's target endpoint %s is in DoNotReuseList, will not add it.", sock->OShandle(), epstr.str());
-                sock->shutdown();
-                sock->close();
+                shutdownAndCloseNoThrow(sock);
                 return;
                 return;
             }
             }
         }
         }

+ 1 - 2
esp/platform/espplugin.cpp

@@ -112,8 +112,7 @@ int CEspServiceThread::run()
 
 
    DBGLOG("Closing Socket(%d)", m_socket->OShandle());
    DBGLOG("Closing Socket(%d)", m_socket->OShandle());
 
 
-   m_socket->shutdown();
-   m_socket->close();
+   shutdownAndCloseNoThrow(m_socket);
 
 
    Release();
    Release();
    return 0;
    return 0;

+ 4 - 8
esp/test/httptest/httptest.cpp

@@ -886,10 +886,8 @@ int COneServerHttpProxyThread::start()
         if(httptest_tracelevel > 5)
         if(httptest_tracelevel > 5)
             fprintf(m_ofile, ">>sent the response back to %s:%d:\n", peername, port);
             fprintf(m_ofile, ">>sent the response back to %s:%d:\n", peername, port);
 
 
-        socket2->shutdown();
-        socket2->close();
-        m_client->shutdown();
-        m_client->close();
+        shutdownAndCloseNoThrow(socket2);
+        shutdownAndCloseNoThrow(m_client);
     }
     }
     catch(IException *excpt)
     catch(IException *excpt)
     {
     {
@@ -1092,10 +1090,8 @@ int CHttpProxyThread::run()
             //m_client->write(respbuf.str(), respbuf.length());
             //m_client->write(respbuf.str(), respbuf.length());
             fflush(m_ofile);
             fflush(m_ofile);
 
 
-            m_remotesocket->shutdown();
-            m_remotesocket->close();
-            m_client->shutdown();
-            m_client->close();
+            shutdownAndCloseNoThrow(m_remotesocket);
+            shutdownAndCloseNoThrow(m_client);
         }
         }
     }
     }
     catch(IException *excpt)
     catch(IException *excpt)

+ 1 - 2
esp/tools/soapplus/http.cpp

@@ -1826,8 +1826,7 @@ int HttpClient::sendRequest(StringBuffer& req, IFileIO* request_output, IFileIO*
 
 
         if(!isPersist || iter >= numReq - 1)
         if(!isPersist || iter >= numReq - 1)
         {
         {
-            socket->shutdown();
-            socket->close();
+            shutdownAndCloseNoThrow(socket);
         }
         }
 
 
         unsigned end1 = msTick();
         unsigned end1 = msTick();

+ 1 - 18
fs/dafsclient/rmtclient.cpp

@@ -56,24 +56,7 @@ static ISocket *dummyReadWrite::timeoutreadsock = null; // used to trigger
 
 
 void cleanupDaFsSocket(ISocket *sock)
 void cleanupDaFsSocket(ISocket *sock)
 {
 {
-    if (!sock)
-        return;
-    try
-    {
-        sock->shutdown();
-    }
-    catch (IException *e)
-    {
-        e->Release();
-    }
-    try
-    {
-        sock->close();
-    }
-    catch (IException *e)
-    {
-        e->Release();
-    }
+    shutdownAndCloseNoThrow(sock);
 }
 }
 
 
 
 

+ 1 - 18
roxie/ccd/ccdprotocol.cpp

@@ -274,24 +274,7 @@ public:
 
 
     virtual void cleanupSocket(ISocket *sock)
     virtual void cleanupSocket(ISocket *sock)
     {
     {
-        if (!sock)
-            return;
-        try
-        {
-            sock->shutdown();
-        }
-        catch (IException *e)
-        {
-            e->Release();
-        }
-        try
-        {
-            sock->close();
-        }
-        catch (IException *e)
-        {
-            e->Release();
-        }
+        shutdownAndCloseNoThrow(sock);
     }
     }
 
 
     virtual int run()
     virtual int run()

+ 1 - 1
roxie/ccd/ccdqueue.cpp

@@ -2824,7 +2824,7 @@ public:
         if (running)
         if (running)
         {
         {
             running = false;
             running = false;
-            multicastSocket->close();
+            shutdownAndCloseNoThrow(multicastSocket);
         }
         }
         RoxieReceiverBase::stop();
         RoxieReceiverBase::stop();
     }
     }

+ 4 - 7
roxie/udplib/udptrr.cpp

@@ -145,7 +145,7 @@ class CReceiveManager : implements IReceiveManager, public CInterface
         {
         {
             if (flowSocket)
             if (flowSocket)
             {
             {
-                flowSocket->close();
+                shutdownAndCloseNoThrow(flowSocket);
                 flowSocket->Release();
                 flowSocket->Release();
             }
             }
         }
         }
@@ -444,8 +444,7 @@ class CReceiveManager : implements IReceiveManager, public CInterface
         ~receive_receive_flow() 
         ~receive_receive_flow() 
         {
         {
             running = false;
             running = false;
-            if (flow_socket) 
-                flow_socket->close();
+            shutdownAndCloseNoThrow(flow_socket);
             join();
             join();
         }
         }
 
 
@@ -660,10 +659,8 @@ class CReceiveManager : implements IReceiveManager, public CInterface
             DBGLOG("Total data packets seen = %u OOO(%u) Requests(%u) Permits(%u)", dataPacketsReceived.load(), packetsOOO.load(), flowRequestsReceived.load(), flowRequestsSent.load());
             DBGLOG("Total data packets seen = %u OOO(%u) Requests(%u) Permits(%u)", dataPacketsReceived.load(), packetsOOO.load(), flowRequestsReceived.load(), flowRequestsSent.load());
 
 
             running = false;
             running = false;
-            if (receive_socket)
-                receive_socket->close();
-            if (selfFlowSocket)
-                selfFlowSocket->close();
+            shutdownAndCloseNoThrow(receive_socket);
+            shutdownAndCloseNoThrow(selfFlowSocket);
             join();
             join();
             ::Release(receive_socket);
             ::Release(receive_socket);
             ::Release(selfFlowSocket);
             ::Release(selfFlowSocket);

+ 1 - 2
roxie/udplib/udptrs.cpp

@@ -807,8 +807,7 @@ class CSendManager : implements ISendManager, public CInterface
         ~send_receive_flow() 
         ~send_receive_flow() 
         {
         {
             running = false;
             running = false;
-            if (flow_socket)
-                flow_socket->shutdownNoThrow();
+            shutdownAndCloseNoThrow(flow_socket);
             join();
             join();
         }
         }
         
         

+ 1 - 1
system/hrpc/hrpcsock.cpp

@@ -369,7 +369,7 @@ public:
         CriticalBlock critblock(critsect);
         CriticalBlock critblock(critsect);
         if (datasock ) {
         if (datasock ) {
             try {
             try {
-                datasock->shutdown();
+                datasock->shutdownNoThrow();
                 datasock->close();
                 datasock->close();
             }
             }
             catch (IJSOCK_Exception *e)
             catch (IJSOCK_Exception *e)

+ 17 - 9
system/jlib/jsocket.cpp

@@ -5545,15 +5545,7 @@ CSingletonSocketConnection::CSingletonSocketConnection(SocketEndpoint &_ep)
 
 
 CSingletonSocketConnection::~CSingletonSocketConnection()
 CSingletonSocketConnection::~CSingletonSocketConnection()
 {
 {
-    try {
-        if (sock)
-            sock->close();
-    }
-    catch (IException *e) {
-        if (e->errorCode()!=JSOCKERR_graceful_close)
-            EXCLOG(e,"CSingletonSocketConnection close");
-        e->Release();
-    }
+    shutdownAndCloseNoThrow(sock);
 }
 }
 
 
 void CSingletonSocketConnection::set_keep_alive(bool keepalive)
 void CSingletonSocketConnection::set_keep_alive(bool keepalive)
@@ -7106,5 +7098,21 @@ IAllowListHandler *createAllowListHandler(AllowListPopulateFunction populateFunc
     return new CAllowListHandler(populateFunc, roleFormatFunc);
     return new CAllowListHandler(populateFunc, roleFormatFunc);
 }
 }
 
 
+extern jlib_decl void shutdownAndCloseNoThrow(ISocket * optSocket)
+{
+    if (!optSocket)
+        return;
+
+    optSocket->shutdownNoThrow();
+    try
+    {
+        optSocket->close();
+    }
+    catch (IException * e)
+    {
+        e->Release();
+    }
+}
+
 static_assert(sizeof(IpAddress) == 16, "check size of IpAddress");
 static_assert(sizeof(IpAddress) == 16, "check size of IpAddress");
 static_assert(sizeof(SocketEndpoint) == 20, "check size of SocketEndpoint");
 static_assert(sizeof(SocketEndpoint) == 20, "check size of SocketEndpoint");

+ 2 - 0
system/jlib/jsocket.hpp

@@ -730,5 +730,7 @@ public:
     virtual void cancel();
     virtual void cancel();
 };
 };
 
 
+extern jlib_decl void shutdownAndCloseNoThrow(ISocket * optSocket);     // Safely shutdown and close a socket without throwing an exception.
+
 #endif
 #endif
 
 

+ 1 - 1
system/mp/mpcomm.cpp

@@ -2081,7 +2081,7 @@ void CMPConnectThread::checkSelfDestruct(void *p,size32_t sz)
     PROGLOG("MP Self destruct invoked");
     PROGLOG("MP Self destruct invoked");
     try {
     try {
         if (listensock) {
         if (listensock) {
-            listensock->close();
+            shutdownAndCloseNoThrow(listensock);
             listensock->Release();
             listensock->Release();
             listensock=NULL;
             listensock=NULL;
         }
         }

+ 1 - 9
system/security/securesocket/securesocket.cpp

@@ -2059,15 +2059,7 @@ public:
 
 
     virtual ~CSingletonSecureSocketConnection()
     virtual ~CSingletonSecureSocketConnection()
     {
     {
-        try {
-            if (sock)
-                sock->close();
-        }
-        catch (IException *e) {
-            if (e->errorCode()!=JSOCKERR_graceful_close)
-                EXCLOG(e,"CSingletonSocketConnection close");
-            e->Release();
-        }
+        shutdownAndCloseNoThrow(sock);
     }
     }
 
 
     bool connect(unsigned timeoutms) override
     bool connect(unsigned timeoutms) override

+ 1 - 11
thorlcr/msort/tsorts1.cpp

@@ -420,17 +420,7 @@ public:
             }
             }
             e->Release();
             e->Release();
         }
         }
-        try {
-            server->close();
-        }
-        catch (IJSOCK_Exception *e)
-        {
-            if (e->errorCode()!=JSOCKERR_cancel_accept) {
-                PrintExceptionLog(e,"CSortTransferServerThread closing");
-                // Ignore for now
-            }
-            e->Release();
-        }
+        shutdownAndCloseNoThrow(server);
         subjoin();
         subjoin();
         DBGLOG("CSortTransferServerThread finished");
         DBGLOG("CSortTransferServerThread finished");
         return 0;
         return 0;