Skip to content

Commit

Permalink
Revert "Fix ETW Sink Initialize unproperly locking" (microsoft#21360)
Browse files Browse the repository at this point in the history
Reverts microsoft#21226

Causes any onnxruntime app to hang on Windows ARM64. Our pipelines do
not have the same ETW environment, so we couldn't catch it.


![image](https://github.com/user-attachments/assets/80edbf7d-be50-4cb0-a016-f390b81dc798)
The call to TraceLoggingRegisterEx() recursively calls back into
LazyInitialize():
LazyInitialize() -> TraceLoggingRegisterEx() ->
ORT_TL_EtwEnableCallback() -> Instance() -> LazyInitialize()

The original code got out of the recursive loop by checking the
`initialized_` flag.
  • Loading branch information
adrianlizarraga authored Jul 16, 2024
1 parent 50170c6 commit cf565e9
Show file tree
Hide file tree
Showing 2 changed files with 23 additions and 3 deletions.
22 changes: 19 additions & 3 deletions onnxruntime/core/platform/windows/logging/etw_sink.cc
Original file line number Diff line number Diff line change
Expand Up @@ -98,6 +98,10 @@ 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 @@ -140,9 +144,15 @@ EtwRegistrationManager::EtwRegistrationManager() {
}

void EtwRegistrationManager::LazyInitialize() {
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));
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_));
}
}
}
}

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

} // namespace logging
Expand Down

0 comments on commit cf565e9

Please sign in to comment.