More fixes

src/Common/Exception.cpp

@@ -3,6 +3,7 @@
 #include <IO/ReadBufferFromString.h>
 #include <IO/ReadHelpers.h>
 #include <IO/WriteHelpers.h>
+#include <Loggers/OwnSplitChannel.h>
 #include <base/demangle.h>
 #include <Common/AtomicLogger.h>
 #include <Common/ErrorCodes.h>
@@ -100,7 +101,7 @@ Exception::Exception(const MessageMasked & msg_masked, int code, bool remote_)
 {
     if (terminate_on_any_exception)
         std::_Exit(terminate_status_code);
-    capture_thread_frame_pointers = thread_frame_pointers;
+    capture_thread_frame_pointers = getThreadFramePointers();
     handle_error_code(msg_masked.msg, code, remote, getStackFramePointers());
 }
 
@@ -110,7 +111,7 @@ Exception::Exception(MessageMasked && msg_masked, int code, bool remote_)
 {
     if (terminate_on_any_exception)
         std::_Exit(terminate_status_code);
-    capture_thread_frame_pointers = thread_frame_pointers;
+    capture_thread_frame_pointers = getThreadFramePointers();
     handle_error_code(message(), code, remote, getStackFramePointers());
 }
 
@@ -119,7 +120,7 @@ Exception::Exception(CreateFromPocoTag, const Poco::Exception & exc)
 {
     if (terminate_on_any_exception)
         std::_Exit(terminate_status_code);
-    capture_thread_frame_pointers = thread_frame_pointers;
+    capture_thread_frame_pointers = getThreadFramePointers();
 #ifdef STD_EXCEPTION_HAS_STACK_TRACE
     auto * stack_trace_frames = exc.get_stack_trace_frames();
     auto stack_trace_size = exc.get_stack_trace_size();
@@ -133,7 +134,7 @@ Exception::Exception(CreateFromSTDTag, const std::exception & exc)
 {
     if (terminate_on_any_exception)
         std::_Exit(terminate_status_code);
-    capture_thread_frame_pointers = thread_frame_pointers;
+    capture_thread_frame_pointers = getThreadFramePointers();
 #ifdef STD_EXCEPTION_HAS_STACK_TRACE
     auto * stack_trace_frames = exc.get_stack_trace_frames();
     auto stack_trace_size = exc.get_stack_trace_size();
@@ -223,10 +224,38 @@ Exception::FramePointers Exception::getStackFramePointers() const
 }
 
 thread_local bool Exception::enable_job_stack_trace = false;
-thread_local std::vector<StackTrace::FramePointers> Exception::thread_frame_pointers = {};
+thread_local bool Exception::can_use_thread_frame_pointers = false;
+thread_local Exception::ThreadFramePointers Exception::thread_frame_pointers;
+
+Exception::ThreadFramePointers::ThreadFramePointers()
+{
+    can_use_thread_frame_pointers = true;
+}
+
+Exception::ThreadFramePointers::~ThreadFramePointers()
+{
+    can_use_thread_frame_pointers = false;
+}
+
+Exception::ThreadFramePointersBase Exception::getThreadFramePointers()
+{
+    if (can_use_thread_frame_pointers)
+        return thread_frame_pointers.frame_pointers;
+
+    return {};
+}
+
+void Exception::setThreadFramePointers(ThreadFramePointersBase frame_pointers)
+{
+    if (can_use_thread_frame_pointers)
+        thread_frame_pointers.frame_pointers = std::move(frame_pointers);
+}
 
 static void tryLogCurrentExceptionImpl(Poco::Logger * logger, const std::string & start_of_message)
 {
+    if (!OwnSplitChannel::isLoggingEnabled())
+        return;
+
     try
     {
         PreformattedMessage message = getCurrentExceptionMessageAndPattern(true);
@@ -242,6 +271,9 @@ static void tryLogCurrentExceptionImpl(Poco::Logger * logger, const std::string
 
 void tryLogCurrentException(const char * log_name, const std::string & start_of_message)
 {
+    if (!OwnSplitChannel::isLoggingEnabled())
+        return;
+
     /// Under high memory pressure, new allocations throw a
     /// MEMORY_LIMIT_EXCEEDED exception.
     ///

src/Common/Exception.h

@@ -10,7 +10,6 @@
 
 #include <cerrno>
 #include <exception>
-#include <memory>
 #include <vector>
 
 #include <fmt/core.h>
@@ -49,14 +48,14 @@ public:
     {
         if (terminate_on_any_exception)
             std::terminate();
-        capture_thread_frame_pointers = thread_frame_pointers;
+        capture_thread_frame_pointers = getThreadFramePointers();
     }
 
     Exception(const PreformattedMessage & msg, int code): Exception(msg.text, code)
     {
         if (terminate_on_any_exception)
             std::terminate();
-        capture_thread_frame_pointers = thread_frame_pointers;
+        capture_thread_frame_pointers = getThreadFramePointers();
         message_format_string = msg.format_string;
         message_format_string_args = msg.format_string_args;
     }
@@ -65,18 +64,36 @@ public:
     {
         if (terminate_on_any_exception)
             std::terminate();
-        capture_thread_frame_pointers = thread_frame_pointers;
+        capture_thread_frame_pointers = getThreadFramePointers();
         message_format_string = msg.format_string;
         message_format_string_args = msg.format_string_args;
     }
 
     /// Collect call stacks of all previous jobs' schedulings leading to this thread job's execution
     static thread_local bool enable_job_stack_trace;
-    static thread_local std::vector<StackTrace::FramePointers> thread_frame_pointers;
+    static thread_local bool can_use_thread_frame_pointers;
+    /// Because of unknown order of static destructor calls,
+    /// thread_frame_pointers can already be uninitialized when a different destructor generates an exception.
+    /// To prevent such scenarios, a wrapper class is created and a function that will return empty vector
+    /// if its destructor is already called
+    using ThreadFramePointersBase = std::vector<StackTrace::FramePointers>;
+    struct ThreadFramePointers
+    {
+        ThreadFramePointers();
+        ~ThreadFramePointers();
+
+        ThreadFramePointersBase frame_pointers;
+    };
+
+    static ThreadFramePointersBase getThreadFramePointers();
+    static void setThreadFramePointers(ThreadFramePointersBase frame_pointers);
+
     /// Callback for any exception
     static std::function<void(const std::string & msg, int code, bool remote, const Exception::FramePointers & trace)> callback;
 
 protected:
+    static thread_local ThreadFramePointers thread_frame_pointers;
+
     // used to remove the sensitive information from exceptions if query_masking_rules is configured
     struct MessageMasked
     {
@@ -178,7 +195,7 @@ class ErrnoException : public Exception
 public:
     ErrnoException(std::string && msg, int code, int with_errno) : Exception(msg, code), saved_errno(with_errno)
     {
-        capture_thread_frame_pointers = thread_frame_pointers;
+        capture_thread_frame_pointers = getThreadFramePointers();
         addMessage(", {}", errnoToString(saved_errno));
     }
 
@@ -187,7 +204,7 @@ public:
     requires std::is_convertible_v<T, String>
     ErrnoException(int code, T && message) : Exception(message, code), saved_errno(errno)
     {
-        capture_thread_frame_pointers = thread_frame_pointers;
+        capture_thread_frame_pointers = getThreadFramePointers();
         addMessage(", {}", errnoToString(saved_errno));
     }
 

src/Common/SignalHandlers.cpp

@@ -89,7 +89,7 @@ void signalHandler(int sig, siginfo_t * info, void * context)
     writePODBinary(*info, out);
     writePODBinary(signal_context, out);
     writePODBinary(stack_trace, out);
-    writeVectorBinary(Exception::enable_job_stack_trace ? Exception::thread_frame_pointers : std::vector<StackTrace::FramePointers>{}, out);
+    writeVectorBinary(Exception::enable_job_stack_trace ? Exception::getThreadFramePointers() : std::vector<StackTrace::FramePointers>{}, out);
     writeBinary(static_cast<UInt32>(getThreadId()), out);
     writePODBinary(current_thread, out);
 

src/Common/StackTrace.cpp

@@ -489,7 +489,7 @@ struct CacheEntry
 
 using CacheEntryPtr = std::shared_ptr<CacheEntry>;
 
-static constinit std::atomic<bool> can_use_cache = false;
+static constinit bool can_use_cache = false;
 
 using StackTraceCacheBase = std::map<StackTraceTriple, CacheEntryPtr, std::less<>>;
 

src/Common/ThreadPool.cpp

@@ -51,7 +51,7 @@ public:
         if (!capture_frame_pointers)
             return;
         /// Save all previous jobs call stacks and append with current
-        frame_pointers = DB::Exception::thread_frame_pointers;
+        frame_pointers = DB::Exception::getThreadFramePointers();
         frame_pointers.push_back(StackTrace().getFramePointers());
     }
 
@@ -455,7 +455,7 @@ void ThreadPoolImpl<Thread>::worker(typename std::list<Thread>::iterator thread_
         try
         {
             if (DB::Exception::enable_job_stack_trace)
-                DB::Exception::thread_frame_pointers = std::move(job_data->frame_pointers);
+                DB::Exception::setThreadFramePointers(std::move(job_data->frame_pointers));
 
             CurrentMetrics::Increment metric_active_pool_threads(metric_active_threads);
 

src/Loggers/OwnSplitChannel.cpp

@@ -18,6 +18,11 @@ namespace DB
 
 static constinit std::atomic<bool> allow_logging{true};
 
+bool OwnSplitChannel::isLoggingEnabled()
+{
+    return allow_logging;
+}
+
 void OwnSplitChannel::disableLogging()
 {
     allow_logging = false;
@@ -25,7 +30,7 @@ void OwnSplitChannel::disableLogging()
 
 void OwnSplitChannel::log(const Poco::Message & msg)
 {
-    if (!allow_logging)
+    if (!isLoggingEnabled())
         return;
 
 #ifndef WITHOUT_TEXT_LOG

src/Loggers/OwnSplitChannel.h

@@ -41,6 +41,8 @@ public:
 
     static void disableLogging();
 
+    static bool isLoggingEnabled();
+
 private:
     void logSplit(const Poco::Message & msg);
     void tryLogSplit(const Poco::Message & msg);