diagnostics: improve logging of hcs helper utilities on debug builds #13971

benhillis · 2025-12-23T21:37:54Z

This change adds additional debug-only tracing to help diagnose HCS / HNS issues.

Copilot

Pull request overview

This PR enhances diagnostic capabilities for HCS/HNS operations by adding debug-only tracing and improving error messages. The changes include adding WSL_LOG_DEBUG calls to key functions, optimizing static data caching with std::call_once, and improving error message formatting.

Key changes:

Added WSL_LOG_DEBUG tracing to 11 HCS/HNS API wrapper functions for better diagnostics
Refactored GetProcessorFeatures() and GetSchemaVersion() to use static caching with std::call_once for thread-safe one-time initialization
Improved error messages by standardizing format specifiers to %ls and splitting long error messages into separate log calls

src/windows/common/hcs.cpp

benhillis · 2025-12-23T22:29:12Z

@copilot open a new pull request to apply changes based on the comments in this thread

Copilot · 2025-12-23T22:29:21Z

@benhillis I've opened a new pull request, #13973, to work on those changes. Once the pull request is ready, I'll request review from you.

…#13973) * Initial plan * Fix code review issues: assign g_schemaVersion and correct ExecutionContext Co-authored-by: benhillis <[email protected]> * Complete code review fixes Co-authored-by: benhillis <[email protected]> --------- Co-authored-by: copilot-swe-agent[bot] <[email protected]> Co-authored-by: benhillis <[email protected]>

Copilot

Pull request overview

Copilot reviewed 1 out of 2 changed files in this pull request and generated 4 comments.

Copilot · 2025-12-23T22:49:36Z

src/windows/common/hcs.cpp

-    ExecutionContext context(Context::HCS);
+    static std::vector<std::string> g_processorFeatures;
+    static std::once_flag flag;
+    std::call_once(flag, [&]() {


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_processorFeatures which is safe to access without capturing.

Suggested change

std::call_once(flag, [&]() {

std::call_once(flag, []() {

Copilot · 2025-12-23T22:49:36Z

src/windows/common/hcs.cpp

-        if (version.Major >= majorVersion)
+    static std::pair<uint32_t, uint32_t> g_schemaVersion{};
+    static std::once_flag flag;
+    std::call_once(flag, [&]() {


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.

Suggested change

std::call_once(flag, [&]() {

std::call_once(flag, []() {

Copilot · 2025-12-23T22:49:37Z

src/windows/common/hcs.cpp

+    static std::vector<std::string> g_processorFeatures;
+    static std::once_flag flag;
+    std::call_once(flag, [&]() {
+        ExecutionContext context(Context::HCS);

-    wil::unique_cotaskmem_string result;
-    THROW_IF_FAILED(::HcsGetServiceProperties(c_processorCapabilitiesQuery, &result));
+        wil::unique_cotaskmem_string result;
+        THROW_IF_FAILED(::HcsGetServiceProperties(c_processorCapabilitiesQuery, &result));

-    const auto properties = wsl::shared::FromJson<ServicePropertiesResponse<PropertyResponse<ProcessorCapabilitiesInfo>>>(result.get());
+        const auto properties =
+            wsl::shared::FromJson<ServicePropertiesResponse<PropertyResponse<ProcessorCapabilitiesInfo>>>(result.get());

-    const auto& response = properties.PropertyResponses.at(c_processorCapabilities);
-    if (response.Error)
-    {
-        THROW_HR_MSG(static_cast<HRESULT>(response.Error->Error), "%hs", response.Error->ErrorMessage.c_str());
-    }
+        const auto& response = properties.PropertyResponses.at(c_processorCapabilities);
+        if (response.Error)
+        {
+            THROW_HR_MSG(static_cast<HRESULT>(response.Error->Error), "%hs", response.Error->ErrorMessage.c_str());
+        }
+
+        g_processorFeatures = std::move(response.Response.ProcessorFeatures);
+    });

-    return response.Response.ProcessorFeatures;
+    return g_processorFeatures;


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.

Copilot · 2025-12-23T22:49:37Z

src/windows/common/hcs.cpp

+    static std::pair<uint32_t, uint32_t> g_schemaVersion{};
+    static std::once_flag flag;
+    std::call_once(flag, [&]() {
+        ExecutionContext context(Context::HCS);
+
+        PropertyQuery query;
+        query.PropertyTypes.emplace_back(PropertyType::Basic);
+        wil::unique_cotaskmem_string result;
+        THROW_IF_FAILED(::HcsGetServiceProperties(wsl::shared::ToJsonW(query).c_str(), &result));
+
+        const auto properties = wsl::shared::FromJson<ServiceProperties<BasicInformation>>(result.get());
+        THROW_HR_IF_MSG(E_UNEXPECTED, properties.Properties.empty(), "%ls", result.get());
+
+        uint32_t majorVersion = 0;
+        uint32_t minorVersion = 0;
+        for (const auto& version : properties.Properties[0].SupportedSchemaVersions)
        {
-            if ((version.Major > majorVersion) || (version.Minor > minorVersion))
+            if (version.Major >= majorVersion)
            {
-                majorVersion = version.Major;
-                minorVersion = version.Minor;
+                if ((version.Major > majorVersion) || (version.Minor > minorVersion))
+                {
+                    majorVersion = version.Major;
+                    minorVersion = version.Minor;
+                }
            }
        }
-    }

-    return {majorVersion, minorVersion};
+        g_schemaVersion = {majorVersion, minorVersion};
+    });
+
+    return g_schemaVersion;


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.

benhillis · 2025-12-24T00:18:29Z

@copilot open a new pull request to apply changes based on the comments in this thread

Copilot · 2025-12-24T00:18:38Z

@benhillis I've opened a new pull request, #13977, to work on those changes. Once the pull request is ready, I'll request review from you.

OneBlue · 2025-12-24T02:48:35Z

src/windows/common/hcs.cpp

+            THROW_HR_MSG(static_cast<HRESULT>(response.Error->Error), "%hs", response.Error->ErrorMessage.c_str());
+        }
+
+        g_processorFeatures = std::move(response.Response.ProcessorFeatures);


nit: this doesn't actually move the structure since response is a const reference

OneBlue · 2025-12-24T02:49:10Z

src/windows/common/hcs.cpp

 std::vector<std::string> wsl::windows::common::hcs::GetProcessorFeatures()
 {


now that we're keeping a static copy, we could return a const reference to avoid copying the vector every call

OneBlue · 2025-12-24T02:50:17Z

src/windows/common/hcs.cpp


 wsl::shared::hns::HNSEndpoint wsl::windows::common::hcs::GetEndpointProperties(HCN_ENDPOINT Endpoint)
 {
+    WSL_LOG_DEBUG("HcsGetEndpointProperties");


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

(Same for below calls)

The idea is on debug builds it will give a better idea of the sequence of calls, not sure what fails.

Im playing with this a bit more, will adjust.

diagnostics: improve logging of hcs helper utilities on debug builds

28e2a4e

benhillis requested a review from a team as a code owner December 23, 2025 21:37

benhillis requested review from Brian-Perkins, OneBlue, Copilot, craigloewen-msft and justus-camp-microsoft December 23, 2025 21:37

Copilot started reviewing on behalf of benhillis December 23, 2025 21:38 View session

Copilot AI reviewed Dec 23, 2025

View reviewed changes

src/windows/common/hcs.cpp Show resolved Hide resolved

src/windows/common/hcs.cpp Outdated Show resolved Hide resolved

Copilot AI mentioned this pull request Dec 23, 2025

Fix missing assignment and incorrect context in hcs.cpp debug logging #13973

Merged

5 tasks

Copilot AI and others added 2 commits December 23, 2025 14:35

remove _codeql_detected_source_root and add to .gitignore

d1d7aac

benhillis requested a review from Copilot December 23, 2025 22:47

Copilot started reviewing on behalf of benhillis December 23, 2025 22:47 View session

Copilot AI reviewed Dec 23, 2025

View reviewed changes

Copilot AI mentioned this pull request Dec 24, 2025

Fix lambda captures and exception handling in HCS static initialization #13977

Closed

5 tasks

OneBlue reviewed Dec 24, 2025

View reviewed changes

		std::vector<std::string> wsl::windows::common::hcs::GetProcessorFeatures()
		{

diagnostics: improve logging of hcs helper utilities on debug builds #13971

Are you sure you want to change the base?

diagnostics: improve logging of hcs helper utilities on debug builds #13971

Conversation

benhillis commented Dec 23, 2025

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

benhillis commented Dec 23, 2025

Uh oh!

Copilot AI commented Dec 23, 2025

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI Dec 23, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 23, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 23, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 23, 2025

Choose a reason for hiding this comment

Uh oh!

benhillis commented Dec 24, 2025

Uh oh!

Copilot AI commented Dec 24, 2025

Uh oh!

OneBlue Dec 24, 2025

Choose a reason for hiding this comment

Uh oh!

OneBlue Dec 24, 2025

Choose a reason for hiding this comment

Uh oh!

OneBlue Dec 24, 2025

Choose a reason for hiding this comment

Uh oh!

OneBlue Dec 24, 2025

Choose a reason for hiding this comment

Uh oh!

benhillis Dec 24, 2025

Choose a reason for hiding this comment

Uh oh!

benhillis Dec 24, 2025

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants