LoaderAllocator selection logic for a generic type instance does not account for a LoaderAllocator of type itself

[alexey-zakharov]

Description

ClassLoader::ComputeLoaderModuleWorker uses a LoaderAllocator creation number to determine the latest LoaderAllocator to place a class in. However the method does not account the number of the type definition itself when calculating the latest number and this can lead into a situation when the generic instance is put into an older LoaderAllocator.

E.g. if we have type ClassA from AssemblyLoadContext A with number 2 and type ClassB from AssemblyLoadContext B with number 5 the resulting loader allocator for the type ClassB would correspond to AssemblyLoadContext A with creation number 2. If ClassB contains a static, such static would be then placed under the LoaderAllocator A, preventing AssemblyLoadContext B from unloading unless A is unloaded.

Reproduction Steps

1. Create 2 classes in 2 different assemblies

Assembly A

public class ClassA
{
}

Assembly B

public class ClassB<T>
{
    public static T instance = new T();
}

2. Load A.dll into a collectible AssemblyLoadContext A.
3. Load B.dll into a collectible AssemblyLoadContext B.
4. Instantiate type ClassB<ClassA>.
5. Ensure instance is not referenced and unload AssemblyLoadContext B.

Expected behavior

AssemblyLoadContext B is unloaded successfully

Actual behavior

AssemblyLoadContext B is not unloaded

Regression?

No

Known Workarounds

No response

Configuration

No response

Other information

The following change seem to fix the issue - Unity-Technologies#279

[dotnet-policy-service]

Tagging subscribers to this area: @vitek-karas, @agocke, @VSadov
See info in area-owners.md if you want to be subscribed.

[jkotas]

cc @davidwrighton

[davidwrighton]

@alexey-zakharov this looks like a good idea. Would you like to open a PR with your change against the dotnet/runtime repo? I think it is a nice upgrade to the existing logic, and I would happily approve it.

[alexey-zakharov]

@alexey-zakharov this looks like a good idea. Would you like to open a PR with your change against the dotnet/runtime repo? I think it is a nice upgrade to the existing logic, and I would happily approve it.

awesome, thanks @davidwrighton! I'm happy to make a PR - this is it with small cleanups #111706