Delay destruction of mutexes #26204
Delay destruction of mutexes #26204
Conversation
Move function-local static mutexes to file-scope statics. This is to delay the destruction of mutexes, as in libc++ the destructor is non-trivial. When a function-local static variable with a non-trivial destructor is defined, the compiler generates code to register its destruction at program exit. This can lead to order of destruction issues, especially in a multi-threaded environment. By moving the mutex to a file-scope static, we ensure that it is initialized at program startup and its destructor is called at program exit, but in a more controlled manner. This avoids potential race conditions and other issues related to the order of destruction of static variables.
snnn
force-pushed
the
fix/mutex-destruction-order
branch
from
4ab9ac7 to
8ba0fce
Compare
|
Will get a macOS machine and test this change. |
|
I don't know what problem to fix ... |
| #endif | ||
|
|
||
| #ifndef ORT_CONSTINIT | ||
| #define ORT_CONSTINIT |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
3 participants
Move function-local static mutexes to file-scope statics.
This is to delay the destruction of mutexes, as in libc++ the destructor is non-trivial.
When a function-local static variable with a non-trivial destructor is defined, the compiler generates code to register its destruction at program exit. This can lead to order of destruction issues. By moving the mutex to a file-scope static, we ensure that it is initialized at program startup and its destructor is called at program exit, but in a more controlled manner. This avoids potential race conditions and other issues related to the order of destruction of static variables. The construction and destruction of std::mutex do not have any dependency other than the standard C++ runtime, therefore this change is safe.
Previously we made the mutexes function local because before VS 17.10 std::mutex's constructor was not constexpr(they cannot be initialized at compile time). So, it would cause constructor order problems if we didn't make them function local.
This PR replaces #25770 .