From efa496c4cdb8ea540d93ec5b4e95ca8a0f7baee1 Mon Sep 17 00:00:00 2001 From: owent Date: Tue, 24 Dec 2024 15:57:59 +0800 Subject: [PATCH 1/3] Try to fix lifetime of GlobalLogHandler --- .../sdk/common/global_log_handler.h | 35 +++++++++---------- 1 file changed, 17 insertions(+), 18 deletions(-) diff --git a/sdk/include/opentelemetry/sdk/common/global_log_handler.h b/sdk/include/opentelemetry/sdk/common/global_log_handler.h index eadd408762..c33062756b 100644 --- a/sdk/include/opentelemetry/sdk/common/global_log_handler.h +++ b/sdk/include/opentelemetry/sdk/common/global_log_handler.h @@ -142,24 +142,23 @@ OPENTELEMETRY_END_NAMESPACE * To ensure that GlobalLogHandler is the first one to be initialized (and so last to be * destroyed), it is first used inside the constructors of TraceProvider, MeterProvider * and LoggerProvider for debug logging. */ -#define OTEL_INTERNAL_LOG_DISPATCH(level, message, attributes) \ - do \ - { \ - using opentelemetry::sdk::common::internal_log::GlobalLogHandler; \ - using opentelemetry::sdk::common::internal_log::LogHandler; \ - if (level > GlobalLogHandler::GetLogLevel()) \ - { \ - break; \ - } \ - const opentelemetry::nostd::shared_ptr &log_handler = \ - GlobalLogHandler::GetLogHandler(); \ - if (!log_handler) \ - { \ - break; \ - } \ - std::stringstream tmp_stream; \ - tmp_stream << message; \ - log_handler->Handle(level, __FILE__, __LINE__, tmp_stream.str().c_str(), attributes); \ +#define OTEL_INTERNAL_LOG_DISPATCH(level, message, attributes) \ + do \ + { \ + using opentelemetry::sdk::common::internal_log::GlobalLogHandler; \ + using opentelemetry::sdk::common::internal_log::LogHandler; \ + if (level > GlobalLogHandler::GetLogLevel()) \ + { \ + break; \ + } \ + opentelemetry::nostd::shared_ptr log_handler = GlobalLogHandler::GetLogHandler(); \ + if (!log_handler) \ + { \ + break; \ + } \ + std::stringstream tmp_stream; \ + tmp_stream << message; \ + log_handler->Handle(level, __FILE__, __LINE__, tmp_stream.str().c_str(), attributes); \ } while (false); #define OTEL_INTERNAL_LOG_GET_3RD_ARG(arg1, arg2, arg3, ...) arg3 From 1dba9526050dd0aee88b22561e36f084009df435 Mon Sep 17 00:00:00 2001 From: owent Date: Tue, 31 Dec 2024 20:03:13 +0800 Subject: [PATCH 2/3] Ignore global handle when singlethon is destroyed --- .../sdk/common/global_log_handler.h | 41 ++++++++++++++++--- sdk/src/common/global_log_handler.cc | 11 +++-- 2 files changed, 43 insertions(+), 9 deletions(-) diff --git a/sdk/include/opentelemetry/sdk/common/global_log_handler.h b/sdk/include/opentelemetry/sdk/common/global_log_handler.h index c33062756b..5ced1ad18e 100644 --- a/sdk/include/opentelemetry/sdk/common/global_log_handler.h +++ b/sdk/include/opentelemetry/sdk/common/global_log_handler.h @@ -93,15 +93,36 @@ class NoopLogHandler : public LogHandler */ class GlobalLogHandler { +private: + struct GlobalLogHandlerData + { + nostd::shared_ptr handler; + LogLevel log_level; + bool destroyed; + + ~GlobalLogHandlerData(); + + GlobalLogHandlerData(const GlobalLogHandlerData &) = delete; + GlobalLogHandlerData(GlobalLogHandlerData &&) = delete; + + GlobalLogHandlerData &operator=(const GlobalLogHandlerData &) = delete; + GlobalLogHandlerData &operator=(GlobalLogHandlerData &&) = delete; + }; + public: /** * Returns the singleton LogHandler. * * By default, a default LogHandler is returned. */ - static inline const nostd::shared_ptr &GetLogHandler() noexcept + static inline nostd::shared_ptr GetLogHandler() noexcept { - return GetHandlerAndLevel().first; + if OPENTELEMETRY_UNLIKELY_CONDITION (GetHandlerAndLevel().destroyed) + { + return nostd::shared_ptr(); + } + + return GetHandlerAndLevel().handler; } /** @@ -111,7 +132,12 @@ class GlobalLogHandler */ static inline void SetLogHandler(const nostd::shared_ptr &eh) noexcept { - GetHandlerAndLevel().first = eh; + if OPENTELEMETRY_UNLIKELY_CONDITION (GetHandlerAndLevel().destroyed) + { + return; + } + + GetHandlerAndLevel().handler = eh; } /** @@ -119,17 +145,20 @@ class GlobalLogHandler * * By default, a default log level is returned. */ - static inline LogLevel GetLogLevel() noexcept { return GetHandlerAndLevel().second; } + static inline LogLevel GetLogLevel() noexcept { return GetHandlerAndLevel().log_level; } /** * Changes the singleton Log level. * This should be called once at the start of application before creating any Provider * instance. */ - static inline void SetLogLevel(LogLevel level) noexcept { GetHandlerAndLevel().second = level; } + static inline void SetLogLevel(LogLevel level) noexcept + { + GetHandlerAndLevel().log_level = level; + } private: - static std::pair, LogLevel> &GetHandlerAndLevel() noexcept; + static GlobalLogHandlerData &GetHandlerAndLevel() noexcept; }; } // namespace internal_log diff --git a/sdk/src/common/global_log_handler.cc b/sdk/src/common/global_log_handler.cc index f60527eee8..741be179ae 100644 --- a/sdk/src/common/global_log_handler.cc +++ b/sdk/src/common/global_log_handler.cc @@ -57,10 +57,15 @@ void NoopLogHandler::Handle(LogLevel, const sdk::common::AttributeMap &) noexcept {} -std::pair, LogLevel> &GlobalLogHandler::GetHandlerAndLevel() noexcept +GlobalLogHandler::GlobalLogHandlerData::~GlobalLogHandlerData() { - static std::pair, LogLevel> handler_and_level{ - nostd::shared_ptr(new DefaultLogHandler), LogLevel::Warning}; + destroyed = true; +} + +GlobalLogHandler::GlobalLogHandlerData &GlobalLogHandler::GetHandlerAndLevel() noexcept +{ + static GlobalLogHandlerData handler_and_level{ + nostd::shared_ptr(new DefaultLogHandler), LogLevel::Warning, false}; return handler_and_level; } From 2bb008b1623c1000f5ecf107f2a5e30594122990 Mon Sep 17 00:00:00 2001 From: owent Date: Tue, 31 Dec 2024 20:17:11 +0800 Subject: [PATCH 3/3] Fix compiling --- .../opentelemetry/sdk/common/global_log_handler.h | 1 + sdk/src/common/global_log_handler.cc | 9 +++++++-- 2 files changed, 8 insertions(+), 2 deletions(-) diff --git a/sdk/include/opentelemetry/sdk/common/global_log_handler.h b/sdk/include/opentelemetry/sdk/common/global_log_handler.h index 5ced1ad18e..55cc333d25 100644 --- a/sdk/include/opentelemetry/sdk/common/global_log_handler.h +++ b/sdk/include/opentelemetry/sdk/common/global_log_handler.h @@ -100,6 +100,7 @@ class GlobalLogHandler LogLevel log_level; bool destroyed; + GlobalLogHandlerData(); ~GlobalLogHandlerData(); GlobalLogHandlerData(const GlobalLogHandlerData &) = delete; diff --git a/sdk/src/common/global_log_handler.cc b/sdk/src/common/global_log_handler.cc index 741be179ae..c350667c62 100644 --- a/sdk/src/common/global_log_handler.cc +++ b/sdk/src/common/global_log_handler.cc @@ -57,6 +57,12 @@ void NoopLogHandler::Handle(LogLevel, const sdk::common::AttributeMap &) noexcept {} +GlobalLogHandler::GlobalLogHandlerData::GlobalLogHandlerData() + : handler(nostd::shared_ptr(new DefaultLogHandler)), + log_level(LogLevel::Warning), + destroyed(false) +{} + GlobalLogHandler::GlobalLogHandlerData::~GlobalLogHandlerData() { destroyed = true; @@ -64,8 +70,7 @@ GlobalLogHandler::GlobalLogHandlerData::~GlobalLogHandlerData() GlobalLogHandler::GlobalLogHandlerData &GlobalLogHandler::GetHandlerAndLevel() noexcept { - static GlobalLogHandlerData handler_and_level{ - nostd::shared_ptr(new DefaultLogHandler), LogLevel::Warning, false}; + static GlobalLogHandlerData handler_and_level; return handler_and_level; }