[main] Fix LoaderAllocator computation for a generic type instance

src/coreclr/vm/clsload.cpp

@@ -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:
 //
@@ -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];
@@ -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];
@@ -155,14 +154,18 @@ PTR_Module ClassLoader::ComputeLoaderModuleWorker(
     if (FALSE)
     {
 ComputeCollectibleLoaderModule:
-        LoaderAllocator *pLoaderAllocatorOfDefiningType = NULL;
-        LoaderAllocator *pOldestLoaderAllocator = NULL;
         Module *pOldestLoaderModule = NULL;
-        UINT64 oldestFoundAge = 0;
+        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())
+        {
+            pOldestLoaderModule = pDefinitionModule;
+            latestFoundNumber = pDefinitionModule->GetLoaderAllocator()->GetCreationNumber();
+        }
 
         for (DWORD i = 0; i < totalArgsCount; i++) {
 
@@ -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();
+                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
+        // Use the module of the latest found collectible loader allocator.
+        // If nothing was found, then by default we use the defining module.
         if (pOldestLoaderModule != NULL)
             pLoaderModule = pOldestLoaderModule;
-        else
-            pLoaderModule = pDefinitionModule;
     }
     RETURN PTR_Module(pLoaderModule);
 }