Fix race condition in ProviderNameToGuid causing ERROR_INSUFFICIENT_BUFFER crashes

[Copilot]

ProviderNameToGuid calls TdhEnumerateProviders twice: once to get buffer size, once to enumerate. If providers register/unregister between these calls, the second call fails with HR 122 (ERROR_INSUFFICIENT_BUFFER), causing EnableProvider to crash intermittently.

Changes

• Added retry loop mimicking GetEnabledProvidersForSession pattern that handles buffer size changes between calls
• Optimized to single API call per iteration - start with null/0 values, let API update buffSize on HR=122
• Validate API calls - throw descriptive errors for non-122 failures
• Prevent NullReferenceException - always assign providersByName (even if empty) to s_providersByName to avoid null reference issues on subsequent lookups
• Test Added - added ProviderNameToGuidTests.cs that validates GetProviderGuidByName doesn't throw and returns the correct GUID for ClrTraceEventParser.ProviderName (Microsoft-Windows-DotNETRuntime), matching ClrTraceEventParser.ProviderGuid

Pattern

// Before: Single attempt, crashes on buffer size change
hr = TdhEnumerateProviders(null, ref buffSize);
Debug.Assert(hr == 122);
buffer = stackalloc byte[buffSize];
hr = TdhEnumerateProviders(providersDesc, ref buffSize);
if (hr != 0) throw;

// After: Retry loop handles race condition
for (; ; )
{
    if (buffSize > 0)
    {
        buffer = stackalloc byte[buffSize];
        providersDesc = buffer;
    }
    hr = TdhEnumerateProviders(providersDesc, ref buffSize);
    if (hr == 0) break;
    if (hr != 122) throw;
    // Loop retries if hr == 122
}
// Always assign result (even if empty) to prevent NullReferenceException
s_providersByName = providersByName;

Testing

• ✅ Built successfully with no warnings or errors
• ✅ Maintains same API contract and behavior
• ✅ Adds robustness against race conditions
• ✅ Prevents null reference issues on edge cases
• ✅ New test validates GetProviderGuidByName works correctly and doesn't throw

Original prompt

---

This section details on the original issue you should resolve

<issue_title>[TraceEvent] EnableProvider crashing with HR 122 ERROR_INSUFFICIENT_BUFFER</issue_title>
<issue_description>In dotnet/runtime, a basic EventSource test has been failing intermittently for a while with System.NullReferenceException : Object reference not set to an instance of an object.

 at Microsoft.Diagnostics.Tracing.Session.TraceEventProviders.GetProviderGuidByName(String name)
   at Microsoft.Diagnostics.Tracing.Session.TraceEventSession.EnableProvider(String providerName, TraceEventLevel providerLevel, UInt64 matchAnyKeywords, TraceEventProviderOptions options)
   at BasicEventSourceTests.EtwListener.EventSourceCommand(String eventSourceName, EventCommand command, FilteringOptions options) in /_/src/libraries/System.Diagnostics.Tracing/tests/BasicEventSourceTest/Harness/EtwListener.cs:line 74
   at BasicEventSourceTests.TestsUserErrors.Test_Bad_EventSource_Startup(Boolean onStartup, Listener listener, EventSourceSettings settings) in /_/src/libraries/System.Diagnostics.Tracing/tests/BasicEventSourceTest/TestsUserErrors.cs:line 110
   at BasicEventSourceTests.TestsUserErrors.Test_BadEventSource_MismatchedIds_WithEtwListener() in /_/src/libraries/System.Diagnostics.Tracing/tests/BasicEventSourceTest/TestsUserErrors.Etw.cs:line 48
   at System.RuntimeMethodHandle.InvokeMethod(Object target, Void** arguments, Signature sig, Boolean isConstructor)
   at System.Reflection.MethodBaseInvoker.InvokeWithNoArgs(Object obj, BindingFlags invokeAttr) in /_/src/libraries/System.Private.CoreLib/src/System/Reflection/MethodBaseInvoker.cs:line 57

Once the TraceEvent version was bumped in the runtime, the exception thrown became System.Exception : TdhEnumerateProviders failed HR = 122 which I was able to connect to #2177, confirming the ERROR_INSUFFICIENT_BUFFER alluded to in that PR.

cc: @cincuranet @brianrob </issue_description>

Comments on the Issue (you are @copilot in this section)

@brianrob Thanks for reporting @mdh1418. I think the issue is that it's technically possible for the buffer size to change in-between the two calls to TdhEnumerateProviders inside of ProviderNameToGuid and so we need to be able to handle that. I'm thinking that we should mimic the behavior of TraceEventSession.GetEnabledProvidersForSession in that it calls the method in a for-loop and only bails out of the loop if hr == 0 || hr != 122. If the hr is another value then we should throw.

• Fixes [TraceEvent] EnableProvider crashing with HR 122 ERROR_INSUFFICIENT_BUFFER #2354

---

✨ Let Copilot coding agent set things up for you — coding agent works faster and does higher quality work when set up for your repo.

[brianrob]

@copilot please address this.

[brianrob]

We should be able to fold this call into the one below it. We're always guaranteed to get an hr=122 here but after we get one, if we go through the loop again, we don't need to call again with null and a 0 buffSize. Rather, let's call the first time where providerDesc is NULL and buffSize is 0 and then each time they should take on the values that come back if hr=122.

[Copilot]

Done. The code now makes a single call to TdhEnumerateProviders per iteration, starting with providersDesc=null and buffSize=0 on the first call. The API updates buffSize when it returns 122, which is then used for subsequent iterations. (commit 0e6fadb)

[brianrob]

@copilot two more comments.

[brianrob]

You should not need another local variable. This should just be directly assigned to buffer.

[Copilot]

I kept the intermediate space variable because C# requires it for assigning stackalloc to a pointer (direct assignment causes error CS8346). This matches the pattern in GetEnabledProvidersForSession (lines 2571-2572). Is there a way to avoid the intermediate variable that I'm missing?

[leculver]

This shouldn't be using stackalloc at all inside of a loop. How big can buffSize get? Using stackalloc at all is likely over-optimization here. We only ever run this code once (and ensure that with a lock) surely just allocating on the heap shouldn't cause too much trouble here, and will ensure easier to follow code, just use Array.Resize or similar.

[brianrob]

If after all this, hr == 0 but providersDesc is null, then we should just assign providersByName (now an empty dictionary) to s_providersByName so that any subsequent lookups fail, but they don't throw a NullReferenceException.

[Copilot]

Done. Now always assigns providersByName to s_providersByName (even if empty when providersDesc is null) to avoid NullReferenceException on subsequent lookups. (commit 226f120)

[brianrob]

@copilot, this looks good. Let's add a new test case that calls ProviderNameToGuid and ensures that it doesn't throw. Then, it should get the GUID for Microsoft-Windows-DotNetRuntime whose constant name is in ClrTraceEventParser.ProviderName and make sure that it matches the GUID in ClrTraceEventParser.ProviderGuid.

[Copilot]

@copilot, this looks good. Let's add a new test case that calls ProviderNameToGuid and ensures that it doesn't throw. Then, it should get the GUID for Microsoft-Windows-DotNetRuntime whose constant ...

Done. Added ProviderNameToGuidTests.cs that calls GetProviderGuidByName with ClrTraceEventParser.ProviderName (Microsoft-Windows-DotNETRuntime) and verifies it doesn't throw and returns the correct GUID matching ClrTraceEventParser.ProviderGuid. (commit 394deab)