diff --git a/src/Common/Exception.cpp b/src/Common/Exception.cpp index 33befa64946..c4bd4fbd943 100644 --- a/src/Common/Exception.cpp +++ b/src/Common/Exception.cpp @@ -3,6 +3,7 @@ #include #include #include +#include #include #include #include @@ -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 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. /// diff --git a/src/Common/Exception.h b/src/Common/Exception.h index 4e54c411bf1..a4f55f41caa 100644 --- a/src/Common/Exception.h +++ b/src/Common/Exception.h @@ -10,7 +10,6 @@ #include #include -#include #include #include @@ -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 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; + struct ThreadFramePointers + { + ThreadFramePointers(); + ~ThreadFramePointers(); + + ThreadFramePointersBase frame_pointers; + }; + + static ThreadFramePointersBase getThreadFramePointers(); + static void setThreadFramePointers(ThreadFramePointersBase frame_pointers); + /// Callback for any exception static std::function 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 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)); } diff --git a/src/Common/SignalHandlers.cpp b/src/Common/SignalHandlers.cpp index 52c83d80121..e025e49e0a3 100644 --- a/src/Common/SignalHandlers.cpp +++ b/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{}, out); + writeVectorBinary(Exception::enable_job_stack_trace ? Exception::getThreadFramePointers() : std::vector{}, out); writeBinary(static_cast(getThreadId()), out); writePODBinary(current_thread, out); diff --git a/src/Common/StackTrace.cpp b/src/Common/StackTrace.cpp index ff8765c9727..76277cbc993 100644 --- a/src/Common/StackTrace.cpp +++ b/src/Common/StackTrace.cpp @@ -489,7 +489,7 @@ struct CacheEntry using CacheEntryPtr = std::shared_ptr; -static constinit std::atomic can_use_cache = false; +static constinit bool can_use_cache = false; using StackTraceCacheBase = std::map>; diff --git a/src/Common/ThreadPool.cpp b/src/Common/ThreadPool.cpp index 01f561d573f..c8f1ae99969 100644 --- a/src/Common/ThreadPool.cpp +++ b/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::worker(typename std::list::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); diff --git a/src/Loggers/OwnSplitChannel.cpp b/src/Loggers/OwnSplitChannel.cpp index c0e8514c62a..e29d2a1e0aa 100644 --- a/src/Loggers/OwnSplitChannel.cpp +++ b/src/Loggers/OwnSplitChannel.cpp @@ -18,6 +18,11 @@ namespace DB static constinit std::atomic 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 diff --git a/src/Loggers/OwnSplitChannel.h b/src/Loggers/OwnSplitChannel.h index 9872a4fb558..9de55f330be 100644 --- a/src/Loggers/OwnSplitChannel.h +++ b/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);