Skip to content

Commit

Permalink
Fix ETW Sink Initialize unproperly locking (#21226)
Browse files Browse the repository at this point in the history
### 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
  • Loading branch information
zhangxiang1993 authored Jul 9, 2024
1 parent d1c19e7 commit 1ab162f
Show file tree
Hide file tree
Showing 2 changed files with 3 additions and 23 deletions.
22 changes: 3 additions & 19 deletions onnxruntime/core/platform/windows/logging/etw_sink.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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<OrtMutex> lock(callbacks_mutex_);
callbacks_.push_back(&callback);
Expand Down Expand Up @@ -144,15 +140,9 @@ EtwRegistrationManager::EtwRegistrationManager() {
}

void EtwRegistrationManager::LazyInitialize() {
if (!initialized_) {
std::lock_guard<OrtMutex> 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));
}
}

Expand All @@ -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;
Expand Down
4 changes: 0 additions & 4 deletions onnxruntime/core/platform/windows/logging/etw_sink.h
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down Expand Up @@ -100,7 +97,6 @@ class EtwRegistrationManager {
bool is_enabled_;
UCHAR level_;
ULONGLONG keyword_;
HRESULT etw_status_;
};

} // namespace logging
Expand Down

0 comments on commit 1ab162f

Please sign in to comment.