-
Notifications
You must be signed in to change notification settings - Fork 3.5k
[node] Fix logging mutex crash at exit on macOS #26445
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.
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
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR refactors the mutex protecting the default logger by converting it from a function-local static (Meyer's Singleton pattern) to a static class member. The mutex is moved from the DefaultLoggerMutex() function to LoggingManager::default_logger_mutex_.
- Removed the
DefaultLoggerMutex()function and replaced it with a static member variable - Updated all references to use
default_logger_mutex_instead ofDefaultLoggerMutex() - Updated the comment in
CreateDefaultLogger()to reflect the new name
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| onnxruntime/core/common/logging/logging.cc | Replaced function-local static mutex with static class member, updated all usages and comments |
| include/onnxruntime/core/common/logging/logging.h | Added declaration of static mutex member |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
4776c4d to
2134765
Compare
|
Updated the PR to use a totally different implementation. Now the latest change only modify nodejs binding to ensure no |
fix destructor order by using different way to deal with OrtEnv
2134765 to
69c8756
Compare
|
@copilot please re-generate overview based on latest code change and remove the out-of-dated one. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Co-authored-by: Copilot <[email protected]>
Description
This change fixes a bug that causes crash on macOS (and also potentially other platforms using libc) at
OrtReleaseEnv.Now we don't let the destructor of OrtEnv to be called if the program exits unexpectedly.
Motivation and Context
Fixes #24579