Skip to content

Fix type-event cache lifetime race during tracing #134281

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
steveisok merged 2 commits into from
Sep 26, 2026
Merged

Fix type-event cache lifetime race during tracing #134281

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
bedf4e3
Fix type event cache lifetime race
steveisok Sep 19, 2026
3d98a8b
Document type cache allocation locking
steveisok Sep 26, 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
155 changes: 65 additions & 90 deletions src/coreclr/vm/eventtrace.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1503,122 +1503,97 @@ BOOL ETW::TypeSystemLog::AddTypeToGlobalCacheIfNotExists(TypeHandle th, BOOL * p
}
CONTRACTL_END;

BOOL fSucceeded = FALSE;

{
CrstHolder _crst(GetHashCrst());

// Check if ETW is enabled, and if not, bail here.
// We do this inside of the lock to ensure that we don't immediately
// re-allocate the global type hash after it has been cleaned up.
if (!ETW_TRACING_CATEGORY_ENABLED(
MICROSOFT_WINDOWS_DOTNETRUNTIME_PROVIDER_DOTNET_Context,
TRACE_LEVEL_INFORMATION,
CLR_TYPE_KEYWORD))
{
*pfCreatedNew = FALSE;
return fSucceeded;
}
*pfCreatedNew = FALSE;

if (s_pAllLoggedTypes == NULL)
{
s_pAllLoggedTypes = new (nothrow) AllLoggedTypes;
if (s_pAllLoggedTypes == NULL)
{
// out of memory. Bail on ETW stuff
*pfCreatedNew = FALSE;
return fSucceeded;
}
}
}
BOOL fSucceeded = FALSE;

// Step 1: go from LoaderModule to hash of types.
Module * pLoaderModule = th.GetLoaderModule();
_ASSERTE(pLoaderModule != NULL);
LoggedTypesFromModule * pLoggedTypesFromModule = nullptr;
{
CrstHolder _crst(GetHashCrst());
pLoggedTypesFromModule = s_pAllLoggedTypes->allLoggedTypesHash.Lookup(pLoaderModule);
}
TypeLoggingInfo typeLoggingInfoNew(th);
LoggedTypesFromModule * pNewLoggedTypesFromModule = NULL;

if (pLoggedTypesFromModule == NULL)
while (TRUE)
{
pLoggedTypesFromModule = new (nothrow) LoggedTypesFromModule(pLoaderModule);
if (pLoggedTypesFromModule == NULL)
{
// out of memory. Bail on ETW stuff
*pfCreatedNew = FALSE;
return fSucceeded;
}
BOOL fAllocateLoggedTypesFromModule = FALSE;
Comment thread
steveisok marked this conversation as resolved.
Resolved

{
CrstHolder _crst(GetHashCrst());
// recheck if the type has been added by another thread since we last checked above
LoggedTypesFromModule * recheckLoggedTypesFromModule = s_pAllLoggedTypes->allLoggedTypesHash.Lookup(pLoaderModule);
if (recheckLoggedTypesFromModule == NULL)

// Check if ETW is enabled, and if not, bail here.
// We do this inside of the lock to ensure that we don't immediately
// re-allocate the global type hash after it has been cleaned up.
if (!ETW_TRACING_CATEGORY_ENABLED(
MICROSOFT_WINDOWS_DOTNETRUNTIME_PROVIDER_DOTNET_Context,
TRACE_LEVEL_INFORMATION,
CLR_TYPE_KEYWORD))
{
EX_TRY
break;
}

if (s_pAllLoggedTypes == NULL)
{
s_pAllLoggedTypes = new (nothrow) AllLoggedTypes;
if (s_pAllLoggedTypes == NULL)
{
s_pAllLoggedTypes->allLoggedTypesHash.Add(pLoggedTypesFromModule);
fSucceeded = TRUE;
break;
}
EX_SWALLOW_NONTERMINAL
}
else

LoggedTypesFromModule * pLoggedTypesFromModule =
s_pAllLoggedTypes->allLoggedTypesHash.Lookup(pLoaderModule);
if (pLoggedTypesFromModule == NULL)
{
delete pLoggedTypesFromModule;
pLoggedTypesFromModule = recheckLoggedTypesFromModule;
if (pNewLoggedTypesFromModule == NULL)
{
fAllocateLoggedTypesFromModule = TRUE;
}
else
{
EX_TRY
{
s_pAllLoggedTypes->allLoggedTypesHash.Add(pNewLoggedTypesFromModule);
pLoggedTypesFromModule = pNewLoggedTypesFromModule;
pNewLoggedTypesFromModule = NULL;
}
EX_SWALLOW_NONTERMINAL
}
}

if (!fSucceeded)
if (!fAllocateLoggedTypesFromModule && pLoggedTypesFromModule != NULL)
{
*pfCreatedNew = FALSE;
return fSucceeded;
// The cache objects must not be used after releasing this lock because
// provider disable and module unload can delete them under the same lock.
if (pLoggedTypesFromModule->loggedTypesFromModuleHash.Lookup(th).th.IsNull())
{
EX_TRY
{
pLoggedTypesFromModule->loggedTypesFromModuleHash.Add(typeLoggingInfoNew);
fSucceeded = TRUE;
}
EX_SWALLOW_NONTERMINAL
}

break;
}
}
}

// Step 2: From hash of types, see if our TypeHandle is there already
TypeLoggingInfo typeLoggingInfoPreexisting;
{
CrstHolder _crst(GetHashCrst());
typeLoggingInfoPreexisting = pLoggedTypesFromModule->loggedTypesFromModuleHash.Lookup(th);
if (!typeLoggingInfoPreexisting.th.IsNull())
if (!fAllocateLoggedTypesFromModule)
{
// Type is already hashed, so it's already logged, so we don't need to
// log it again.
*pfCreatedNew = FALSE;
return fSucceeded;
break;
}
}

// We haven't logged this type, so we need to continue with this function to
// log it below. Add it to the hash table first so any recursive calls will
// see that this type is already being taken care of
fSucceeded = FALSE;
TypeLoggingInfo typeLoggingInfoNew(th);
{
CrstHolder _crst(GetHashCrst());
// Like above, check if the type has been added from a different thread since we last looked it up.
if (!pLoggedTypesFromModule->loggedTypesFromModuleHash.Lookup(th).th.IsNull())
// Allocate outside the lock, then revalidate and publish the candidate
// under the lock on the next iteration.
pNewLoggedTypesFromModule = new (nothrow) LoggedTypesFromModule(pLoaderModule);
if (pNewLoggedTypesFromModule == NULL)
{
*pfCreatedNew = FALSE;
return fSucceeded;
break;
}
}

EX_TRY
{
pLoggedTypesFromModule->loggedTypesFromModuleHash.Add(typeLoggingInfoNew);
fSucceeded = TRUE;
}
EX_SWALLOW_NONTERMINAL
if (!fSucceeded)
{
*pfCreatedNew = FALSE;
return fSucceeded;
}
} // RELEASE: CrstHolder _crst(GetHashCrst());
delete pNewLoggedTypesFromModule;

*pfCreatedNew = TRUE;
*pfCreatedNew = fSucceeded;
return fSucceeded;
}

Expand Down
Loading