Merge pull request ClickHouse#88732 from ClickHouse/backport/25.8/88668

Backport ClickHouse#88668 to 25.8: wrap `~PooledConnection` in try-catch

Author: mstetsyuk

base/poco/Net/include/Poco/Net/HTTPChunkedStream.h

@@ -45,7 +45,7 @@ namespace Net
         ~HTTPChunkedStreamBuf();
         void close();
 
-        bool isComplete(bool read_from_device_to_check_eof = false);
+        bool isComplete(bool read_from_device_to_check_eof = false) noexcept;
 
     protected:
         int readFromDevice(char * buffer, std::streamsize length);

base/poco/Net/src/HTTPChunkedStream.cpp

@@ -140,7 +140,7 @@ int HTTPChunkedStreamBuf::readFromDevice(char* buffer, std::streamsize length)
 }
 
 
-bool HTTPChunkedStreamBuf::isComplete(bool read_from_device_to_check_eof)
+bool HTTPChunkedStreamBuf::isComplete(bool read_from_device_to_check_eof) noexcept
 {
 	if (read_from_device_to_check_eof)
 	{
@@ -150,7 +150,7 @@ bool HTTPChunkedStreamBuf::isComplete(bool read_from_device_to_check_eof)
 			/// "Unexpected EOF" exception would be thrown
 			readFromDevice(nullptr, 0);
 		}
-		catch (Poco::Net::MessageException &)
+		catch (...)
 		{
 			return false;
 		}

src/Common/HTTPConnectionPool.cpp

@@ -438,38 +438,45 @@ class EndpointConnectionPool : public std::enable_shared_from_this<EndpointConne
 
         ~PooledConnection() override
         {
-            if (bool(response_stream))
+            try
             {
-                if (auto * fixed_steam = dynamic_cast<Poco::Net::HTTPFixedLengthInputStream *>(response_stream))
+                if (bool(response_stream))
                 {
-                    response_stream_completed = fixed_steam->isComplete();
+                    if (auto * fixed_steam = dynamic_cast<Poco::Net::HTTPFixedLengthInputStream *>(response_stream))
+                    {
+                        response_stream_completed = fixed_steam->isComplete();
+                    }
+                    else if (auto * chunked_steam = dynamic_cast<Poco::Net::HTTPChunkedInputStream *>(response_stream))
+                    {
+                        response_stream_completed = chunked_steam->isComplete();
+                    }
+                    else if (auto * http_stream = dynamic_cast<Poco::Net::HTTPInputStream *>(response_stream))
+                    {
+                        response_stream_completed = http_stream->isComplete();
+                    }
+                    else
+                    {
+                        response_stream_completed = false;
+                    }
                 }
-                else if (auto * chunked_steam = dynamic_cast<Poco::Net::HTTPChunkedInputStream *>(response_stream))
-                {
-                    response_stream_completed = chunked_steam->isComplete();
-                }
-                else if (auto * http_stream = dynamic_cast<Poco::Net::HTTPInputStream *>(response_stream))
-                {
-                    response_stream_completed = http_stream->isComplete();
-                }
-                else
-                {
-                    response_stream_completed = false;
-                }
-            }
-            response_stream = nullptr;
-            Session::setSendDataHooks();
-            Session::setReceiveDataHooks();
-            Session::setSendThrottler();
-            Session::setReceiveThrottler();
+                response_stream = nullptr;
+                Session::setSendDataHooks();
+                Session::setReceiveDataHooks();
+                Session::setSendThrottler();
+                Session::setReceiveThrottler();
 
-            group->atConnectionDestroy();
+                group->atConnectionDestroy();
 
-            if (!isExpired)
-                if (auto lock = pool.lock())
-                    lock->atConnectionDestroy(*this);
+                if (!isExpired)
+                    if (auto lock = pool.lock())
+                        lock->atConnectionDestroy(*this);
 
-            CurrentMetrics::sub(metrics.active_count);
+                CurrentMetrics::sub(metrics.active_count);
+            }
+            catch (...)
+            {
+                tryLogCurrentException(__PRETTY_FUNCTION__);
+            }
         }
 
     private:

src/Common/tests/gtest_http_chunked_stream.cpp

@@ -0,0 +1,16 @@
+#include <gtest/gtest.h>
+
+#include <Poco/Net/HTTPChunkedStream.h>
+#include <Poco/Net/HTTPClientSession.h>
+
+
+TEST(HTTPChunkedStreamBuf, IsCompleteHandlesInvalidSocketException)
+{
+    Poco::Net::HTTPClientSession session;
+    Poco::Net::HTTPChunkedStreamBuf buf(session, std::ios::in);
+
+    /// Default-initialized socket throws InvalidSocketException in SocketImpl::receiveBytes,
+    /// which HTTPChunkedStreamBuf::isComplete should swallow and return false.
+    bool complete = buf.isComplete(true);
+    ASSERT_FALSE(complete);
+}