Merge pull request ClickHouse#89347 from ClickHouse/backport/25.8/89174

Backport ClickHouse#89174 to 25.8: Avoid possible data-races due to mutable exceptions while parsing Parquet

Author: azat

base/poco/Foundation/include/Poco/Exception.h

@@ -84,7 +84,7 @@ class Foundation_API Exception : public std::exception
     /// The copy can later be thrown again by
     /// invoking rethrow() on it.
 
-    virtual void rethrow() const;
+    [[noreturn]] virtual void rethrow() const;
     /// (Re)Throws the exception.
     ///
     /// This is useful for temporarily storing a

src/Access/Authentication.h

@@ -38,6 +38,9 @@ struct Authentication
         explicit Require(const String & realm_);
         const String & getRealm() const;
 
+        Require * clone() const override { return new Require(*this); }
+        void rethrow() const override { throw *this; } /// NOLINT(cert-err60-cpp)
+
     private:
         const String realm;
     };

src/Client/ClientBase.cpp

@@ -339,6 +339,9 @@ class LocalFormatError : public Exception
 {
 public:
     using Exception::Exception;
+
+    LocalFormatError * clone() const override { return new LocalFormatError(*this); }
+    void rethrow() const override { throw *this; } /// NOLINT(cert-err60-cpp)
 };
 
 

src/Common/Exception.cpp

@@ -779,4 +779,27 @@ ExecutionStatus ExecutionStatus::fromText(const std::string & data)
     return status;
 }
 
+std::exception_ptr copyMutableException(std::exception_ptr ptr)
+{
+    try
+    {
+        std::rethrow_exception(ptr);
+    }
+    catch (Poco::Exception & e)
+    {
+        try
+        {
+            e.rethrow();
+        }
+        catch (...)
+        {
+            return std::current_exception();
+        }
+    }
+    catch (...)
+    {
+        return std::current_exception();
+    }
+}
+
 }

src/Common/Exception.h

@@ -376,4 +376,8 @@ T current_exception_cast()
     }
 }
 
+/// Return copy of a current exception if it is a Poco::Exception (DB::Exception), since this exception is mutable, and returning reference is unsafe.
+/// And a reference otherwise.
+std::exception_ptr copyMutableException(std::exception_ptr ptr);
+
 }

src/Common/mysqlxx/mysqlxx/Exception.h

@@ -14,6 +14,9 @@ struct Exception : public Poco::Exception
     int errnum() const { return code(); }
     const char * name() const noexcept override { return "mysqlxx::Exception"; }
     const char * className() const noexcept override { return "mysqlxx::Exception"; }
+
+    Exception * clone() const override { return new Exception(*this); }
+    void rethrow() const override { throw *this; } /// NOLINT(cert-err60-cpp)
 };
 
 
@@ -23,6 +26,9 @@ struct ConnectionFailed : public Exception
     explicit ConnectionFailed(const std::string & msg, int code = 0) : Exception(msg, code) {}
     const char * name() const noexcept override { return "mysqlxx::ConnectionFailed"; }
     const char * className() const noexcept override { return "mysqlxx::ConnectionFailed"; }
+
+    ConnectionFailed * clone() const override { return new ConnectionFailed(*this); }
+    void rethrow() const override { throw *this; } /// NOLINT(cert-err60-cpp)
 };
 
 
@@ -32,6 +38,9 @@ struct ConnectionLost : public Exception
     explicit ConnectionLost(const std::string & msg, int code = 0) : Exception(msg, code) {}
     const char * name() const noexcept override { return "mysqlxx::ConnectionLost"; }
     const char * className() const noexcept override { return "mysqlxx::ConnectionLost"; }
+
+    ConnectionLost * clone() const override { return new ConnectionLost(*this); }
+    void rethrow() const override { throw *this; } /// NOLINT(cert-err60-cpp)
 };
 
 
@@ -41,6 +50,9 @@ struct BadQuery : public Exception
     explicit BadQuery(const std::string & msg, int code = 0) : Exception(msg, code) {}
     const char * name() const noexcept override { return "mysqlxx::BadQuery"; }
     const char * className() const noexcept override { return "mysqlxx::BadQuery"; }
+
+    BadQuery * clone() const override { return new BadQuery(*this); }
+    void rethrow() const override { throw *this; } /// NOLINT(cert-err60-cpp)
 };
 
 
@@ -50,6 +62,9 @@ struct CannotParseValue : public Exception
     explicit CannotParseValue(const std::string & msg, int code = 0) : Exception(msg, code) {}
     const char * name() const noexcept override { return "mysqlxx::CannotParseValue"; }
     const char * className() const noexcept override { return "mysqlxx::CannotParseValue"; }
+
+    CannotParseValue * clone() const override { return new CannotParseValue(*this); }
+    void rethrow() const override { throw *this; } /// NOLINT(cert-err60-cpp)
 };
 
 

src/IO/S3Common.h

@@ -47,6 +47,9 @@ class S3Exception : public Exception
 
     bool isRetryableError() const;
 
+    S3Exception * clone() const override { return new S3Exception(*this); }
+    void rethrow() const override { throw *this; } /// NOLINT(cert-err60-cpp)
+
 private:
     Aws::S3::S3Errors code;
 };

src/Processors/Formats/Impl/ParquetBlockInputFormat.cpp

@@ -1169,7 +1169,9 @@ Chunk ParquetBlockInputFormat::read()
         if (background_exception)
         {
             is_stopped = true;
-            std::rethrow_exception(background_exception);
+            /// This exception can be mutated (addMessage()) in IInputFormat::generate(),
+            /// so we need to copy it (copyMutableException()) to avoid sharing mutable exception between multiple threads
+            std::rethrow_exception(copyMutableException(background_exception));
         }
         if (is_stopped)
             return {};