System.Linq.Expressions may prevent assembly unloading when using MethodInfo objects from non-collectible type with collectible generic arguments

[tbdty]

Description

When you create a MethodInfo from a generic method in a non-collectible assembly, then call MakeGenericMethod with a type from a collectible assembly and use the returned MethodInfo in System.Linq.Expressions.Expression.Call the latter assembly will never be unloaded.

Reproduction Steps

Create an assembly SomeCollectibleAssembly with a single class in it:

namespace SomeCollectibleAssembly;

public class SomeType
{ }

Compile this assembly and note the path of the compiled assembly. Then create a console application with the following code:

using System.Linq.Expressions;
using System.Reflection;
using System.Runtime.CompilerServices;
using System.Runtime.Loader;

namespace MinimalRepro;

internal static class Program
{
    private const string ASSEMBLY_PATH = """C:\Path\To\SomeCollectibleAssembly.dll""";
    private const string ASSEMBLY_NAME = "SomeCollectibleAssembly";

    private static void Main(string[] args)
    {
        WorkWithCollectibleAssembly();
        
        while (true)
        {
            if (!IsAssemblyLoaded(ASSEMBLY_NAME))
            {
                Console.WriteLine("The assembly has been unloaded.");
                break;
            }

            Console.WriteLine("The assembly has not been unloaded, collecting ...");
            GC.Collect();
        }
    }

    [MethodImpl(MethodImplOptions.NoInlining)]
    private static void WorkWithCollectibleAssembly()
    {
        var assembly = LoadAssembly();

        var someType = assembly.GetType("SomeCollectibleAssembly.SomeType")
            ?? throw new Exception("SomeType not found");

        var someGenericMethod = typeof(Program).GetMethod(nameof(SomeGenericMethod), BindingFlags.NonPublic | BindingFlags.Static)
            ?? throw new Exception("SomeGenericMethod not found");

        var method = someGenericMethod.MakeGenericMethod(someType);

        // Commenting out this line allows unloading to succeed.
        _ = Expression.Call(method);
    }

    private static Assembly LoadAssembly()
    {
        var alc = new AssemblyLoadContext(null, isCollectible: true);
        try
        {
            return alc.LoadFromAssemblyPath(ASSEMBLY_PATH);
        }
        finally
        {
            alc.Unload();
        }
    }

    [MethodImpl(MethodImplOptions.NoInlining)]
    private static bool IsAssemblyLoaded(string assemblyName)
    {
        return AppDomain.CurrentDomain.GetAssemblies().Any(a => a.GetName().Name == assemblyName);
    }

    private static void SomeGenericMethod<T>() { }
}

Expected behavior

It is expected that the assembly is eventually collected by the GC and unloads, i.e. the reproduction program prints:

The assembly has not been unloaded, collecting ...
The assembly has been unloaded.

Actual behavior

The program never terminates and prints

The assembly has not been unloaded, collecting ...
The assembly has not been unloaded, collecting ...
The assembly has not been unloaded, collecting ...
The assembly has not been unloaded, collecting ...
The assembly has not been unloaded, collecting ...
The assembly has not been unloaded, collecting ...

repeatedly.

Regression?

No response

Known Workarounds

Using a non-generic method in a generic type works around the problem, i.e. changing the program as follows allows unloading to succeed:

using System.Linq.Expressions;
using System.Reflection;
using System.Runtime.CompilerServices;
using System.Runtime.Loader;

namespace MinimalRepro;

internal static class Program
{
    private const string ASSEMBLY_PATH = """C:\Path\To\SomeCollectibleAssembly.dll""";
    private const string ASSEMBLY_NAME = "SomeCollectibleAssembly";

    private static void Main(string[] args)
    {
        WorkWithCollectibleAssembly();
        
        while (true)
        {
            if (!IsAssemblyLoaded(ASSEMBLY_NAME))
            {
                Console.WriteLine("The assembly has been unloaded.");
                break;
            }

            Console.WriteLine("The assembly has not been unloaded, collecting ...");
            GC.Collect();
        }
    }

    [MethodImpl(MethodImplOptions.NoInlining)]
    private static void WorkWithCollectibleAssembly()
    {
        var assembly = LoadAssembly();

        var someType = assembly.GetType("SomeCollectibleAssembly.SomeType")
            ?? throw new Exception("SomeType not found");

        var method = typeof(SomeGenericType<>)
            .MakeGenericType(someType)
            .GetMethod(nameof(SomeGenericType<object>.SomeNonGenericMethod), BindingFlags.Public | BindingFlags.Static)
            ?? throw new Exception("Method not found");

        _ = Expression.Call(method);
    }

    private static Assembly LoadAssembly()
    {
        var alc = new AssemblyLoadContext(null, isCollectible: true);
        try
        {
            return alc.LoadFromAssemblyPath(ASSEMBLY_PATH);
        }
        finally
        {
            alc.Unload();
        }
    }

    [MethodImpl(MethodImplOptions.NoInlining)]
    private static bool IsAssemblyLoaded(string assemblyName)
    {
        return AppDomain.CurrentDomain.GetAssemblies().Any(a => a.GetName().Name == assemblyName);
    }

    private static class SomeGenericType<T>
    {
        public static void SomeNonGenericMethod() { }
    }
}

Configuration

No response

Other information

I have investigated this unloading problem using WinDbg and have determined that the problem is the TypeExtensions.s_paramInfoCache dictionary.

The problem lies in the parameter caching code: Instead of calling method.DeclaringType?.IsCollectible, calling method.IsCollectible here should fix the problem.

[huoyaoyuan]

Related to #71629 and #89536. Generics constructed from different assemblies are not easy to handle.

[dotnet-policy-service]

Tagging subscribers to this area: @cston
See info in area-owners.md if you want to be subscribed.

[jkotas]

Generics constructed from different assemblies are not easy to handle.

System.Linq.Expressions has code to handle this, except that it appears to have a subtle bug as @tbdty pointed out:

The problem lies in the parameter caching code: Instead of calling method.DeclaringType?.IsCollectible, calling method.IsCollectible here should fix the problem.

[jkotas]

@tbdty Would you be interested in submitting a PR with the fix that you have proposed?

[tbdty]

@tbdty Would you be interested in submitting a PR with the fix that you have proposed?

I would, but sadly I currently do not have the spare time to set up a development environment for .NET and ensure that I do everything properly. I'd greatly appreciate if someone else did this.

[tbdty]

I started looking into this. Are there any tests for the unloadability of assemblies? I looked and did not find any.

[jkotas]

The System.Linq.Expressions tests do have some coverage for collectible assemblies, but they do not explicitly verify that everything got collected.

The fix discussed here was actually discussed earlier in https://github.com/dotnet/corefx/pull/25736/files#r155524477 .

I would be ok with the fix without associated test. The tests to verify that everything got collected with collectible assemblies are somewhat complicated.