From 1ab162fbca76a1dbb40ee3aa37a1d04585869081 Mon Sep 17 00:00:00 2001 From: Xiang Zhang Date: Tue, 9 Jul 2024 10:55:41 -0700 Subject: [PATCH] Fix ETW Sink Initialize unproperly locking (#21226) ### Description ETW trace logger is fakely registered as initialized_ is marked as true before the registration is done, causing crashing issue for Lenovo camera application. [Bug 42610244](https://microsoft.visualstudio.com/OS/_workitems/edit/42610244): [Watson Failure] caused by SVCHOSTGROUP_Camera_INVALID_POINTER_READ_c0000005_onnxruntime.dll!onnxruntime::logging::Logger::Log --- .../core/platform/windows/logging/etw_sink.cc | 22 +++---------------- .../core/platform/windows/logging/etw_sink.h | 4 ---- 2 files changed, 3 insertions(+), 23 deletions(-) diff --git a/onnxruntime/core/platform/windows/logging/etw_sink.cc b/onnxruntime/core/platform/windows/logging/etw_sink.cc index b0f9eaf4f62d2..2a74e22850658 100644 --- a/onnxruntime/core/platform/windows/logging/etw_sink.cc +++ b/onnxruntime/core/platform/windows/logging/etw_sink.cc @@ -98,10 +98,6 @@ ULONGLONG EtwRegistrationManager::Keyword() const { return keyword_; } -HRESULT EtwRegistrationManager::Status() const { - return etw_status_; -} - void EtwRegistrationManager::RegisterInternalCallback(const EtwInternalCallback& callback) { std::lock_guard lock(callbacks_mutex_); callbacks_.push_back(&callback); @@ -144,15 +140,9 @@ EtwRegistrationManager::EtwRegistrationManager() { } void EtwRegistrationManager::LazyInitialize() { - if (!initialized_) { - std::lock_guard lock(init_mutex_); - if (!initialized_) { // Double-check locking pattern - initialized_ = true; - etw_status_ = ::TraceLoggingRegisterEx(etw_provider_handle, ORT_TL_EtwEnableCallback, nullptr); - if (FAILED(etw_status_)) { - ORT_THROW("ETW registration failed. Logging will be broken: " + std::to_string(etw_status_)); - } - } + static HRESULT etw_status = ::TraceLoggingRegisterEx(etw_provider_handle, ORT_TL_EtwEnableCallback, nullptr); + if (FAILED(etw_status)) { + ORT_THROW("ETW registration failed. Logging will be broken: " + std::to_string(etw_status)); } } @@ -171,12 +161,6 @@ void EtwSink::SendImpl(const Timestamp& timestamp, const std::string& logger_id, // register on first usage static EtwRegistrationManager& etw_manager = EtwRegistrationManager::Instance(); - // do something (not that meaningful) with etw_manager so it doesn't get optimized out - // as we want an instance around to do the unregister - if (FAILED(etw_manager.Status())) { - return; - } - // TODO: Validate if this filtering makes sense. if (message.DataType() == DataType::USER) { return; diff --git a/onnxruntime/core/platform/windows/logging/etw_sink.h b/onnxruntime/core/platform/windows/logging/etw_sink.h index 3af45b813a625..ff68aec0b7d64 100644 --- a/onnxruntime/core/platform/windows/logging/etw_sink.h +++ b/onnxruntime/core/platform/windows/logging/etw_sink.h @@ -66,9 +66,6 @@ class EtwRegistrationManager { // Get the current keyword uint64_t Keyword() const; - // Get the ETW registration status - HRESULT Status() const; - void RegisterInternalCallback(const EtwInternalCallback& callback); void UnregisterInternalCallback(const EtwInternalCallback& callback); @@ -100,7 +97,6 @@ class EtwRegistrationManager { bool is_enabled_; UCHAR level_; ULONGLONG keyword_; - HRESULT etw_status_; }; } // namespace logging