Skip to content

Delay destruction of mutexes #26204

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

Closed
wants to merge 6 commits into from
Closed

Delay destruction of mutexes #26204

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
8ba0fce
Fix: Delay destruction of mutexes
snnn Sep 29, 2025
3bc87f3
Merge remote-tracking branch 'origin/main' into fix/mutex-destruction…
snnn Oct 7, 2025
31862ec
Add constinit
snnn Oct 7, 2025
f30a80b
Merge remote-tracking branch 'origin/main' into fix/mutex-destruction…
snnn Oct 14, 2025
ae2cb59
Refactor TRT code
snnn Oct 14, 2025
251b24b
update
snnn Oct 14, 2025
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
16 changes: 16 additions & 0 deletions include/onnxruntime/core/common/common.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,22 @@ using TimePoint = std::chrono::high_resolution_clock::time_point;
#define ORT_ATTRIBUTE_UNUSED
#endif

// ORT_CONSTINIT
//
// C++20 constinit keyword ensures that a variable is initialized at compile time
// and can be safely used in static initialization contexts
#if defined(__cpp_constinit) && __cpp_constinit >= 201907L
#define ORT_CONSTINIT constinit
#elif defined(__clang__) && defined(__has_cpp_attribute)
#if __has_cpp_attribute(clang::require_constant_initialization)
#define ORT_CONSTINIT [[clang::require_constant_initialization]]
#endif
#endif

#ifndef ORT_CONSTINIT
#define ORT_CONSTINIT
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Overall, this might work, but we need to resolve this non C++20 case because w/o constinit static mutexes are worse than local function statics.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Currently only macOS platform has this problem, and macOS build is using C++20.
I will continue to work on upgrading all pipelines to use C++20. I would prefer to get this PR merged before that work is done, since a lot of users are waiting for it. The constinit keyword is a sanitize check, which should not impact functionality.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Instead of an empty definition could we instead use #ifdef's in the files? If constinit is available, use that with an #ifdef around the file scope declaration. Otherwise have an #ifdef around the existing function scope declaration.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Then it will have very different behavior on different platforms, since function local statics are deallocated earlier than global vars. Then it will increase the complexity further.

Copy link
Member

@yuslepukhin yuslepukhin Oct 10, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The ideal way to deal with it is to move the mutex into the structure it is trying to protect and not to have it static. I realize it may not be possible in every case, but I can see it is possible in some cases.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For example, in onnxruntime/core/providers/shared_library/provider_bridge_provider.cc, the lifetime of the mutex is shorter than the object that it protects. So, clearly the current code is wrong.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Then it will have very different behavior on different platforms, since function local statics are deallocated earlier than global vars. Then it will increase the complexity further.

Isn't the problem we're trying to address limited to macOS or is that not the case?

For macOS we have C++20. If we use the ifdef's the current behavior on non-C++20 builds is unchanged, and we migrate to constinit automatically as soon as we build with C++20 which should be the safe long term solution.

Is that not more predictable than changing function scope mutexes to file scope for non-C++20 builds?

#endif

#ifdef ORT_NO_EXCEPTIONS
// Print the given final message, the message must be a null terminated char*
// ORT will abort after printing the message.
Expand Down
5 changes: 3 additions & 2 deletions onnxruntime/core/common/logging/logging.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -64,9 +64,10 @@ LoggingManager* LoggingManager::GetDefaultInstance() {
#pragma warning(disable : 26426)
#endif

ORT_CONSTINIT static std::mutex default_logger_mutex;

static std::mutex& DefaultLoggerMutex() noexcept {
static std::mutex mutex;
return mutex;
return default_logger_mutex;
}

Logger* LoggingManager::s_default_logger_ = nullptr;
Expand Down
5 changes: 4 additions & 1 deletion onnxruntime/core/framework/model_metadef_id_generator.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,11 +7,14 @@
#include "core/framework/murmurhash3.h"

namespace onnxruntime {
namespace {
ORT_CONSTINIT static std::mutex mutex;
}

int ModelMetadefIdGenerator::GenerateId(const onnxruntime::GraphViewer& graph_viewer,
HashValue& model_hash) const {
// if the EP is shared across multiple sessions there's a very small potential for concurrency issues.
// use a lock when generating an id to be paranoid
static std::mutex mutex;
std::lock_guard<std::mutex> lock(mutex);
model_hash = 0;

Expand Down
7 changes: 5 additions & 2 deletions onnxruntime/core/providers/nv_tensorrt_rtx/nv_execution_provider.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -290,9 +290,12 @@ TensorrtLogger& GetTensorrtLogger(bool verbose_log) {
return trt_logger;
}

namespace {
ORT_CONSTINIT std::mutex trt_api_lock;
} // namespace

std::unique_lock<std::mutex> NvExecutionProvider::GetApiLock() const {
static std::mutex singleton;
return std::unique_lock<std::mutex>(singleton);
return std::unique_lock<std::mutex>(trt_api_lock);
}

/*
Expand Down
8 changes: 6 additions & 2 deletions onnxruntime/core/providers/nv_tensorrt_rtx/nv_execution_provider_custom_ops.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,11 @@
#endif

namespace onnxruntime {

namespace {
ORT_CONSTINIT static std::mutex trt_custom_op_mutex;
} // namespace

extern TensorrtLogger& GetTensorrtLogger(bool verbose);

/*
Expand All @@ -41,8 +46,7 @@ extern TensorrtLogger& GetTensorrtLogger(bool verbose);
common::Status CreateTensorRTCustomOpDomainList(std::vector<OrtCustomOpDomain*>& domain_list, const std::string extra_plugin_lib_paths) {
static std::unique_ptr<OrtCustomOpDomain> custom_op_domain = std::make_unique<OrtCustomOpDomain>();
static std::vector<std::unique_ptr<TensorRTCustomOp>> created_custom_op_list;
static std::mutex mutex;
std::lock_guard<std::mutex> lock(mutex);
std::lock_guard<std::mutex> lock(trt_custom_op_mutex);
if (custom_op_domain->domain_ != "" && custom_op_domain->custom_ops_.size() > 0) {
domain_list.push_back(custom_op_domain.get());
return Status::OK();
Expand Down
7 changes: 5 additions & 2 deletions onnxruntime/core/providers/qnn/builder/qnn_utils.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -790,16 +790,19 @@ Status GetQnnDataType(const bool is_quantized_tensor, const ONNX_NAMESPACE::Type
return Status::OK();
}

namespace {
ORT_CONSTINIT static std::mutex counter_mutex;
} // namespace

std::string GetUniqueName(const std::string& base, std::string_view suffix) {
std::string name = base;
if (!suffix.empty()) {
name += suffix;
}
{
static std::unordered_map<std::string, int> counter;
static std::mutex counter_mutex;
std::lock_guard<std::mutex> lock(counter_mutex);

static std::unordered_map<std::string, int> counter;
int& count = counter[name];
if (count++ > 0) {
return name + "_" + std::to_string(count);
Expand Down
8 changes: 6 additions & 2 deletions onnxruntime/core/providers/qnn/ort_api.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,11 +10,15 @@
namespace onnxruntime {

#if BUILD_QNN_EP_STATIC_LIB

namespace {
ORT_CONSTINIT static std::mutex run_on_unload_mutex;
} // namespace

static std::unique_ptr<std::vector<std::function<void()>>> s_run_on_unload_;

void RunOnUnload(std::function<void()> function) {
static std::mutex mutex;
std::lock_guard<std::mutex> guard(mutex);
std::lock_guard<std::mutex> guard(run_on_unload_mutex);
if (!s_run_on_unload_) {
s_run_on_unload_ = std::make_unique<std::vector<std::function<void()>>>();
}
Expand Down
8 changes: 6 additions & 2 deletions onnxruntime/core/providers/shared_library/provider_bridge_provider.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -81,6 +81,11 @@ void operator delete(void* p, size_t /*size*/) noexcept { return Provider_GetHos
#endif

namespace onnxruntime {

namespace {
ORT_CONSTINIT static std::mutex run_on_unload_mutex;
} // namespace

#if defined(_MSC_VER) && !defined(__clang__)
#pragma warning(push)
// "Global initializer calls a non-constexpr function."
Expand All @@ -94,8 +99,7 @@ ProviderHostCPU& g_host_cpu = g_host->GetProviderHostCPU();
static std::unique_ptr<std::vector<std::function<void()>>> s_run_on_unload_;

void RunOnUnload(std::function<void()> function) {
static std::mutex mutex;
std::lock_guard<std::mutex> guard{mutex};
std::lock_guard<std::mutex> guard{run_on_unload_mutex};
if (!s_run_on_unload_)
s_run_on_unload_ = std::make_unique<std::vector<std::function<void()>>>();
s_run_on_unload_->push_back(std::move(function));
Expand Down
7 changes: 5 additions & 2 deletions onnxruntime/core/providers/tensorrt/tensorrt_execution_provider.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -452,9 +452,12 @@ TensorrtLogger& GetTensorrtLogger(bool verbose_log) {
return trt_logger;
}

namespace {
ORT_CONSTINIT std::mutex trt_api_lock;
} // namespace

std::unique_lock<std::mutex> TensorrtExecutionProvider::GetApiLock() const {
static std::mutex singleton;
return std::unique_lock<std::mutex>(singleton);
return std::unique_lock<std::mutex>(trt_api_lock);
}

/*
Expand Down
8 changes: 6 additions & 2 deletions onnxruntime/core/providers/tensorrt/tensorrt_execution_provider_custom_ops.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,11 @@
#define ORT_DEF2STR(x) ORT_DEF2STR_HELPER(x)

namespace onnxruntime {

namespace {
ORT_CONSTINIT static std::mutex trt_custom_op_mutex;
} // namespace

extern TensorrtLogger& GetTensorrtLogger(bool verbose);

/*
Expand All @@ -47,8 +52,7 @@ extern TensorrtLogger& GetTensorrtLogger(bool verbose);
common::Status CreateTensorRTCustomOpDomainList(std::vector<OrtCustomOpDomain*>& domain_list, const std::string extra_plugin_lib_paths) {
static std::unique_ptr<OrtCustomOpDomain> custom_op_domain = std::make_unique<OrtCustomOpDomain>();
static std::vector<std::unique_ptr<TensorRTCustomOp>> created_custom_op_list;
static std::mutex mutex;
std::lock_guard<std::mutex> lock(mutex);
std::lock_guard<std::mutex> lock(trt_custom_op_mutex);
if (custom_op_domain->domain_ != "" && custom_op_domain->custom_ops_.size() > 0) {
domain_list.push_back(custom_op_domain.get());
return Status::OK();
Expand Down
Loading