Skip to content

Fix LoaderAllocator computation for a generic type instance #279

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
merged 2 commits into from
Jan 29, 2025
Merged

Fix LoaderAllocator computation for a generic type instance #279

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
e14fa9b
Take defining type loader allocator into account when computing loade…
alexey-zakharov Jan 17, 2025
4e55fbb
Addressed codereview comment
alexey-zakharov Jan 27, 2025
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
57 changes: 28 additions & 29 deletions src/coreclr/vm/clsload.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -78,14 +78,14 @@ NameHandle::NameHandle(Module* pModule, mdToken token) :
// This method determines the "loader module" for an instantiated type
// or method. The rule must ensure that any types involved in the
// instantiated type or method do not outlive the loader module itself
// with respect to app-domain unloading (e.g. MyList<MyType> can't be
// with respect to module unloading (e.g. MyList<MyType> can't be
// put in the module of MyList if MyList's assembly is
// app-domain-neutral but MyType's assembly is app-domain-specific).
// non-collectible but MyType's assembly is collectible).
// The rule we use is:
//
// * Pick the first type in the class instantiation, followed by
// method instantiation, whose loader module is non-shared (app-domain-bound)
// * If no type is app-domain-bound, return the module containing the generic type itself
// method instantiation, whose loader allocator is collectible and has the highest creation number.
// * If no type is in collectible assembly, return the module containing the generic type itself.
//
// Some useful effects of this rule (for ngen purposes) are:
//
Expand Down Expand Up @@ -113,18 +113,15 @@ PTR_Module ClassLoader::ComputeLoaderModuleWorker(
}
CONTRACT_END

// No generic instantiation, return the definition module
if (classInst.IsEmpty() && methodInst.IsEmpty())
RETURN PTR_Module(pDefinitionModule);

Module *pLoaderModule = NULL;

if (pDefinitionModule)
{
if (pDefinitionModule->IsCollectible())
goto ComputeCollectibleLoaderModule;
pLoaderModule = pDefinitionModule;
}
// Use the definition module as the loader module by default
Module *pLoaderModule = pDefinitionModule;

// If any of generic type arguments are in collectible module,
// we use a generic procedure.
for (DWORD i = 0; i < classInst.GetNumArgs(); i++)
{
TypeHandle classArg = classInst[i];
Expand All @@ -135,6 +132,8 @@ PTR_Module ClassLoader::ComputeLoaderModuleWorker(
pLoaderModule = pModule;
}

// If any of generic method arguments are in collectible module,
// we also use a generic procedure.
for (DWORD i = 0; i < methodInst.GetNumArgs(); i++)
{
TypeHandle methodArg = methodInst[i];
Expand All @@ -155,14 +154,18 @@ PTR_Module ClassLoader::ComputeLoaderModuleWorker(
if (FALSE)
{
ComputeCollectibleLoaderModule:
LoaderAllocator *pLoaderAllocatorOfDefiningType = NULL;
LoaderAllocator *pOldestLoaderAllocator = NULL;
Module *pOldestLoaderModule = NULL;
UINT64 oldestFoundAge = 0;
Module *pLatestLoaderModule = NULL;
UINT64 latestFoundNumber = 0;
DWORD classArgsCount = classInst.GetNumArgs();
DWORD totalArgsCount = classArgsCount + methodInst.GetNumArgs();

if (pDefinitionModule != NULL) pLoaderAllocatorOfDefiningType = pDefinitionModule->GetLoaderAllocator();
// If loader allocator of the defining type is collectible, we use it
// and its creation number as the starting age.
if (pDefinitionModule != NULL && pDefinitionModule->IsCollectible())
{
pLatestLoaderModule = pDefinitionModule;
latestFoundNumber = pDefinitionModule->GetLoaderAllocator()->GetCreationNumber();
}

for (DWORD i = 0; i < totalArgsCount; i++) {

Expand All @@ -176,22 +179,18 @@ PTR_Module ClassLoader::ComputeLoaderModuleWorker(
Module *pModuleCheck = arg.GetLoaderModule();
LoaderAllocator *pLoaderAllocatorCheck = pModuleCheck->GetLoaderAllocator();

if (pLoaderAllocatorCheck != pLoaderAllocatorOfDefiningType &&
pLoaderAllocatorCheck->IsCollectible() &&
pLoaderAllocatorCheck->GetCreationNumber() > oldestFoundAge)
if (pLoaderAllocatorCheck->IsCollectible() &&
pLoaderAllocatorCheck->GetCreationNumber() > latestFoundNumber)
{
pOldestLoaderModule = pModuleCheck;
pOldestLoaderAllocator = pLoaderAllocatorCheck;
oldestFoundAge = pLoaderAllocatorCheck->GetCreationNumber();
pLatestLoaderModule = pModuleCheck;
latestFoundNumber = pLoaderAllocatorCheck->GetCreationNumber();
}
}

// Only if we didn't find a different loader allocator than the defining loader allocator do we
// use the defining loader allocator
if (pOldestLoaderModule != NULL)
pLoaderModule = pOldestLoaderModule;
else
pLoaderModule = pDefinitionModule;
// Use the module of the latest found collectible loader allocator.
// If nothing was found, then by default we use the defining module.
if (pLatestLoaderModule != NULL)
pLoaderModule = pLatestLoaderModule;
}
RETURN PTR_Module(pLoaderModule);
}
Expand Down