-
Notifications
You must be signed in to change notification settings - Fork 1.6k
diagnostics: improve logging of hcs helper utilities on debug builds #13971
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
base: master
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
|
|
@@ -78,6 +78,8 @@ wsl::windows::common::hcs::unique_hcs_operation wsl::windows::common::hcs::Creat | |||||
|
|
||||||
| wsl::windows::common::hcs::unique_hcs_system wsl::windows::common::hcs::CreateComputeSystem(_In_ PCWSTR Id, _In_ PCWSTR Configuration) | ||||||
| { | ||||||
| WSL_LOG_DEBUG("HcsCreateComputeSystem", TraceLoggingValue(Id, "id"), TraceLoggingValue(Configuration, "configuration")); | ||||||
|
|
||||||
| ExecutionContext context(Context::HCS); | ||||||
|
|
||||||
| const unique_hcs_operation operation = CreateOperation(); | ||||||
|
|
@@ -98,32 +100,39 @@ wsl::windows::common::hcs::unique_hcs_system wsl::windows::common::hcs::CreateCo | |||||
|
|
||||||
| std::vector<std::string> wsl::windows::common::hcs::GetProcessorFeatures() | ||||||
| { | ||||||
| ExecutionContext context(Context::HCS); | ||||||
| static std::vector<std::string> g_processorFeatures; | ||||||
| static std::once_flag flag; | ||||||
| std::call_once(flag, [&]() { | ||||||
|
||||||
| std::call_once(flag, [&]() { | |
| std::call_once(flag, []() { |
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.
nit: this doesn't actually move the structure since response is a const reference
Copilot
AI
Dec 23, 2025
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.
The function returns a copy of g_processorFeatures by value, but if an exception is thrown during the std::call_once initialization, the static vector will remain in an uninitialized state. Subsequent calls will return the empty vector instead of retrying the initialization. Consider storing an optional or using a different pattern to handle initialization failures.
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.
nit: I'm not sure how much we want to log this since it doesn't carry any specific information, and the below logline will display the full failure context
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.
(Same for below calls)
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.
The idea is on debug builds it will give a better idea of the sequence of calls, not sure what fails.
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.
Im playing with this a bit more, will adjust.
Copilot
AI
Dec 23, 2025
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.
The lambda in std::call_once captures by reference ([&]) but doesn't use any variables from the enclosing scope. This should use an empty capture list ([]) or [=] to avoid potential issues. The lambda only modifies the static variable g_schemaVersion which is safe to access without capturing.
| std::call_once(flag, [&]() { | |
| std::call_once(flag, []() { |
benhillis marked this conversation as resolved.
Show resolved
Hide resolved
Copilot
AI
Dec 23, 2025
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.
The function returns g_schemaVersion by value, but if an exception is thrown during the std::call_once initialization, the static pair will remain in its default-initialized state (0, 0). Subsequent calls will return (0, 0) instead of retrying the initialization. Consider storing an optional or using a different pattern to handle initialization failures.
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.
now that we're keeping a static copy, we could return a const reference to avoid copying the vector every call