Skip to content

Fix race condition in ProviderNameToGuid causing ERROR_INSUFFICIENT_BUFFER crashes #2357

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.

Sign up for GitHub

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

Merged
brianrob merged 9 commits into from
Jan 23, 2026
Merged

Fix race condition in ProviderNameToGuid causing ERROR_INSUFFICIENT_BUFFER crashes #2357

Changes from 4 commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
02659ce
Initial plan
Copilot Jan 23, 2026
76093ea
Fix ERROR_INSUFFICIENT_BUFFER in ProviderNameToGuid by adding retry loop
Copilot Jan 23, 2026
88bfa5d
Address code review: improve error handling and messages
Copilot Jan 23, 2026
01f45a6
Fix comment clarity in retry loop
Copilot Jan 23, 2026
0e6fadb
Optimize retry loop to use single API call per iteration
Copilot Jan 23, 2026
226f120
Always assign providersByName to avoid NullReferenceException
Copilot Jan 23, 2026
394deab
Add test for ProviderNameToGuid to verify it doesn't throw
Copilot Jan 23, 2026
2953988
Clarify test comment to reference GetProviderGuidByName
Copilot Jan 23, 2026
21dc21e
Switch from stackalloc to a heap allocation.
brianrob Jan 23, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
41 changes: 31 additions & 10 deletions src/TraceEvent/TraceEventSession.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1760,13 +1760,38 @@ internal static SortedDictionary<string, Guid> ProviderNameToGuid
{
SortedDictionary<string, Guid> providersByName = new SortedDictionary<string, Guid>(StringComparer.OrdinalIgnoreCase);
int buffSize = 0;
var hr = TraceEventNativeMethods.TdhEnumerateProviders(null, ref buffSize);
Debug.Assert(hr == 122); // ERROR_INSUFFICIENT_BUFFER
var buffer = stackalloc byte[buffSize];
var providersDesc = (TraceEventNativeMethods.PROVIDER_ENUMERATION_INFO*)buffer;
byte* buffer;
TraceEventNativeMethods.PROVIDER_ENUMERATION_INFO* providersDesc;
int hr;

hr = TraceEventNativeMethods.TdhEnumerateProviders(providersDesc, ref buffSize);
if ((hr == 0) && (providersDesc != null))
// Retry loop to handle the case where the buffer size changes between calls
// This can happen if providers are registered/unregistered between the two calls
for (; ; )
{
hr = TraceEventNativeMethods.TdhEnumerateProviders(null, ref buffSize);

@brianrob brianrob Jan 23, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 AI Jan 23, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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)

if (hr != 122) // ERROR_INSUFFICIENT_BUFFER
{
throw new Exception("Failed to get buffer size for provider enumeration. TdhEnumerateProviders failed HR = " + hr);
}

var space = stackalloc byte[buffSize];
buffer = space;
providersDesc = (TraceEventNativeMethods.PROVIDER_ENUMERATION_INFO*)buffer;

hr = TraceEventNativeMethods.TdhEnumerateProviders(providersDesc, ref buffSize);
if (hr == 0)
{
break;
}

// Error 122 means buffer not big enough. For that error we retry, everything else simply fail.
if (hr != 122)
{
throw new Exception("Failed to enumerate trace providers. TdhEnumerateProviders failed HR = " + hr);
}
}

if (providersDesc != null)

@brianrob brianrob Jan 23, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 AI Jan 23, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

{
var providers = (TraceEventNativeMethods.TRACE_PROVIDER_INFO*)&providersDesc[1];
for (int i = 0; i < providersDesc->NumberOfProviders; i++)
Expand All @@ -1777,10 +1802,6 @@ internal static SortedDictionary<string, Guid> ProviderNameToGuid

s_providersByName = providersByName;
}
else
{
throw new Exception("TdhEnumerateProviders failed HR = " + hr);
}
}
}
}
Expand Down