Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix ETW Sink Initialize unproperly locking (#21226) #21302

Open
wants to merge 1 commit into
base: rel-os-1.17.0
Choose a base branch
from
Open

Fix ETW Sink Initialize unproperly locking (#21226) #21302

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 3 additions & 19 deletions onnxruntime/core/platform/windows/logging/etw_sink.cc
View file
Open in desktop
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 @@ -133,15 +129,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 @@ -160,12 +150,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
View file
Open in desktop
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);

private:
Expand Down Expand Up @@ -98,7 +95,6 @@ class EtwRegistrationManager {
bool is_enabled_;
UCHAR level_;
ULONGLONG keyword_;
HRESULT etw_status_;
};

} // namespace logging
Expand Down
Loading