No way to remove type from ConstructorResolverCache

[TETYYS]

Describe the bug
Some types used with Singularity may be a part of AssemblyLoadContext which holds a set of assemblies which may be then unloaded together with AssemblyLoadContext unless something holds a reference to the assembly or one of its types. https://github.com/dotnet/docs/blob/d2ca43e5e87ca76ad737e586014b82eb7a4c3fc3/docs/standard/assembly/unloadability.md#troubleshoot-unloadability-issues

If you use Singularity to construct a class of an unloadable assembly, Singularity (I assume) puts the compiled expression which does the heavy lifting into the global cache. However, after unloading, the type practically doesn't exist now.

There should be a way to remove a type from ConstructorResolverCache.

To Reproduce
Steps to reproduce the behavior:
Run Release build:

using Microsoft.CodeAnalysis;
using Microsoft.CodeAnalysis.CSharp;
using Singularity;
using System;
using System.Collections.Generic;
using System.IO;
using System.Linq;
using System.Reflection;
using System.Runtime.Loader;

namespace test
{
    class UnloadableLoadContext : AssemblyLoadContext
    {
        public UnloadableLoadContext() : base(true) { }
        protected override Assembly Load(AssemblyName assemblyName) => null;
    }

    class Program
    {
        static readonly CSharpParseOptions parseOpts = new CSharpParseOptions(
            kind: SourceCodeKind.Regular,
            languageVersion: LanguageVersion.Latest);
        static readonly CSharpCompilationOptions options = new CSharpCompilationOptions(
            OutputKind.DynamicallyLinkedLibrary,
            optimizationLevel: OptimizationLevel.Release,
            allowUnsafe: false);

        static void Main(string[] args)
        {
            var references = new List<MetadataReference>() {
                MetadataReference.CreateFromFile(typeof(Binder).Assembly.Location),
                MetadataReference.CreateFromFile(AppDomain.CurrentDomain.GetAssemblies().FirstOrDefault(x => x.FullName.Substring(0, x.FullName.IndexOf(',')) == "netstandard").Location),
                MetadataReference.CreateFromFile(AppDomain.CurrentDomain.GetAssemblies().FirstOrDefault(x => x.FullName.Substring(0, x.FullName.IndexOf(',')) == "System.Console").Location),
                MetadataReference.CreateFromFile(AppDomain.CurrentDomain.GetAssemblies().FirstOrDefault(x => x.FullName.Substring(0, x.FullName.IndexOf(',')) == "System.Runtime").Location)
            };

            var csTree = CSharpSyntaxTree.ParseText("class A { public void B() { System.Console.WriteLine(\"hello\"); } }", parseOpts);
            var asmName = Guid.NewGuid().ToString();
            Compilation compilation = CSharpCompilation.Create(asmName, options: options, references: references).AddSyntaxTrees(csTree);

            using (var ms = new MemoryStream()) {
                var emitResult = compilation.Emit(ms);
                ms.Seek(0, SeekOrigin.Begin);

                var alc = new UnloadableLoadContext();
                var alcWeakRef = new WeakReference(alc);
                var asm = alc.LoadFromStream(ms);

                var mthd = asm.GetType("A").GetMethod("B");

                var inst = Activator.CreateInstance(asm.GetType("A"));

                mthd.Invoke(inst, null);
                alc.Unload();

                for (int i = 0;alcWeakRef.IsAlive && (i < 10);i++) {
                    GC.Collect();
                    GC.WaitForPendingFinalizers();
                }

                if (alcWeakRef.IsAlive)
                    Console.WriteLine("[no singularity] noooooooo");
                else
                    Console.WriteLine("[no singularity] all good");
            }

            compilation = CSharpCompilation.Create(asmName, options: options, references: references).AddSyntaxTrees(csTree);

            using (var ms = new MemoryStream()) {
                var emitResult = compilation.Emit(ms);
                ms.Seek(0, SeekOrigin.Begin);

                var alc = new UnloadableLoadContext();
                var alcWeakRef = new WeakReference(alc);
                var asm = alc.LoadFromStream(ms);

                var mthd = asm.GetType("A").GetMethod("B");

                using (var container = new Container()) {
                    using (var scope = container.BeginScope()) {
                        var inst = container.GetInstance(asm.GetType("A"));

                        mthd.Invoke(inst, null);
                    }
                    alc.Unload();
                }

                for (int i = 0;alcWeakRef.IsAlive && (i < 10);i++) {
                    GC.Collect();
                    GC.WaitForPendingFinalizers();
                }

                if (alcWeakRef.IsAlive)
                    Console.WriteLine("[with singularity] noooooooo");
                else
                    Console.WriteLine("[with singularity] all good");
            }
        }
    }
}

Expected behavior

hello
[no singularity] all good
hello
[with singularity] all good

Current behavior

hello
[no singularity] all good
hello
[with singularity] noooooooo

[Barsonax]

Thanks for making a issue about this and providing some code to reproduce this.

You are correct that Singularity uses a global cache here https://github.com/Barsonax/Singularity/blob/master/src/Singularity/Settings/ConstructorResolvers.cs due to static being used. In most cases this is fine except in yours. Contemplating whether to remove the static though and move to a instance based approach as that would make it clearer that something is being cached and in what context.

You can however instantiate the resolvers yourself, thus working around the cache issue so I believe Singularity already supports this scenario.

Some unit tests to confirm this is the case and stays that way wouldn't hurt though.

[TETYYS]

You can however instantiate the resolvers yourself, thus working around the cache issue so I believe Singularity already supports this scenario.

Do you mean creating my own IConstructorResolver? Both Default and BestMatch resolvers are unfortunately read-only in this case

[Barsonax]

No like this, without using the ConstructorResolvers class at all:

var defaultResolver = new ConstructorResolverCache(new DefaultConstructorResolver());

Then give that to the settings or registrations you pass to the container. When the resolver goes out of scope it should clear the cache. You can also remove the cache completely if desired.

If you need a clearer answer. I can make a small code example later when iam at my PC

[TETYYS]

This works.

using Microsoft.CodeAnalysis;
using Microsoft.CodeAnalysis.CSharp;
using Singularity;
using System;
using System.Collections.Generic;
using System.IO;
using System.Linq;
using System.Reflection;
using System.Runtime.Loader;

using Singularity.Exceptions;
using Singularity.Expressions;
using Singularity.Resolving;
using System.Linq.Expressions;
using System.Collections.Concurrent;

namespace test
{
    class UnloadableLoadContext : AssemblyLoadContext
    {
        public UnloadableLoadContext() : base(true) { }
        protected override Assembly Load(AssemblyName assemblyName) => null;
    }

    internal static class TypeExtensions
    {
        public static Expression ResolveConstructorExpression(this Type type, ConstructorInfo? constructor) {
            if (constructor == null && type.IsValueType) return Expression.Default(type);
            else if (constructor == null) throw new ArgumentNullException(nameof(constructor), "You can only pass a null for the constructor argument if the type is a value type");
            ParameterInfo[] parameters = constructor.GetParameters();
            if (parameters.Length == 0) return Expression.New(constructor);
            var parameterExpressions = new Expression[parameters.Length];
            for (var i = 0;i < parameters.Length;i++) parameterExpressions[i] = Expression.Parameter(parameters[i].ParameterType);
            return Expression.New(constructor, parameterExpressions);
        }
    }

    public class DefaultConstructorResolver : IConstructorResolver
    {
        public ConstructorInfo StaticSelectConstructor(Type type) {
            ConstructorInfo[] constructors = type.GetConstructorCandidates().ToArray();
            if (constructors.Length == 0 && !type.IsValueType) throw new Exception($"Type {type} did not contain any public constructor.");
            if (constructors.Length > 1) throw new Exception($"Found {constructors.Length} suitable constructors for type {type}. Please specify the constructor explicitly.");
            return constructors.FirstOrDefault();
        }
        public ConstructorInfo DynamicSelectConstructor(Type type, IInstanceFactoryResolver instanceFactoryResolver) => throw new NotImplementedException();
        public Expression ResolveConstructorExpression(Type type, ConstructorInfo constructorInfo) => type.ResolveConstructorExpression(constructorInfo);
    }

    public class ConstructorResolverCache : IConstructorResolver
    {
        private readonly ConcurrentDictionary<Type, ConstructorInfo?> StaticSelectConstructorCache = new ConcurrentDictionary<Type, ConstructorInfo?>();
        private readonly ConcurrentDictionary<Type, Expression?> ResolveConstructorExpressionCache = new ConcurrentDictionary<Type, Expression?>();
        private readonly IConstructorResolver _constructorResolver;
        public ConstructorResolverCache(IConstructorResolver constructorResolver) { _constructorResolver = constructorResolver; }
        public ConstructorInfo DynamicSelectConstructor(Type type, IInstanceFactoryResolver instanceFactoryResolver) => _constructorResolver.DynamicSelectConstructor(type, instanceFactoryResolver);
        public ConstructorInfo? StaticSelectConstructor(Type type) => StaticSelectConstructorCache.GetOrAdd(type, t => _constructorResolver.StaticSelectConstructor(t));
        public Expression? ResolveConstructorExpression(Type type, ConstructorInfo constructorInfo) => ResolveConstructorExpressionCache.GetOrAdd(type, t => _constructorResolver.ResolveConstructorExpression(t, constructorInfo));

        public void RemoveType(Type type)
        {
            ResolveConstructorExpressionCache.Remove(type, out _);
            StaticSelectConstructorCache.Remove(type, out _);
        }
    }

    class Program
    {
        static readonly CSharpParseOptions parseOpts = new CSharpParseOptions(
            kind: SourceCodeKind.Regular,
            languageVersion: LanguageVersion.Latest);
        static readonly CSharpCompilationOptions options = new CSharpCompilationOptions(
            OutputKind.DynamicallyLinkedLibrary,
            optimizationLevel: OptimizationLevel.Release,
            allowUnsafe: false);

        static ConstructorResolverCache GlobalCache = new ConstructorResolverCache(new DefaultConstructorResolver());

        static void Main(string[] args)
        {
            var references = new List<MetadataReference>() {
                MetadataReference.CreateFromFile(typeof(Binder).Assembly.Location),
                MetadataReference.CreateFromFile(AppDomain.CurrentDomain.GetAssemblies().FirstOrDefault(x => x.FullName.Substring(0, x.FullName.IndexOf(',')) == "netstandard").Location),
                MetadataReference.CreateFromFile(AppDomain.CurrentDomain.GetAssemblies().FirstOrDefault(x => x.FullName.Substring(0, x.FullName.IndexOf(',')) == "System.Console").Location),
                MetadataReference.CreateFromFile(AppDomain.CurrentDomain.GetAssemblies().FirstOrDefault(x => x.FullName.Substring(0, x.FullName.IndexOf(',')) == "System.Runtime").Location)
            };

            var csTree = CSharpSyntaxTree.ParseText("class A { public void B() { System.Console.WriteLine(\"hello\"); } }", parseOpts);
            var asmName = Guid.NewGuid().ToString();
            Compilation compilation = CSharpCompilation.Create(asmName, options: options, references: references).AddSyntaxTrees(csTree);

            using (var ms = new MemoryStream()) {
                var emitResult = compilation.Emit(ms);
                ms.Seek(0, SeekOrigin.Begin);

                var alc = new UnloadableLoadContext();
                var alcWeakRef = new WeakReference(alc);
                var asm = alc.LoadFromStream(ms);

                var mthd = asm.GetType("A").GetMethod("B");

                var inst = Activator.CreateInstance(asm.GetType("A"));

                mthd.Invoke(inst, null);
                alc.Unload();

                for (int i = 0;alcWeakRef.IsAlive && (i < 10);i++) {
                    GC.Collect();
                    GC.WaitForPendingFinalizers();
                }

                if (alcWeakRef.IsAlive)
                    Console.WriteLine("[no singularity] noooooooo");
                else
                    Console.WriteLine("[no singularity] all good");
            }

            compilation = CSharpCompilation.Create(asmName, options: options, references: references).AddSyntaxTrees(csTree);

            using (var ms = new MemoryStream()) {
                var emitResult = compilation.Emit(ms);
                ms.Seek(0, SeekOrigin.Begin);

                var alc = new UnloadableLoadContext();
                var alcWeakRef = new WeakReference(alc);
                var asm = alc.LoadFromStream(ms);

                var type = asm.GetType("A");
                var mthd = type.GetMethod("B");

                using (var container = new Container(builder => {
                    builder.ConfigureSettings(settings => {
                        settings.With(GlobalCache);
                    });
                })) {
                    using (var scope = container.BeginScope()) {
                        var inst = container.GetInstance(type);

                        mthd.Invoke(inst, null);
                    }
                    alc.Unload();
                }

                GlobalCache.RemoveType(type);

                for (int i = 0;alcWeakRef.IsAlive && (i < 10);i++) {
                    GC.Collect();
                    GC.WaitForPendingFinalizers();
                }

                if (alcWeakRef.IsAlive)
                    Console.WriteLine("[with singularity] noooooooo");
                else
                    Console.WriteLine("[with singularity] all good");
            }
        }
    }
}

But as it can be seen, I had to jump through some hoops as some classes or their constructors are internal.

Fun fact - don't name this new ConstructorResolverCache as GlobalResolver as it crashes Visual Studio debugger with message

---------------------------
Microsoft Visual Studio
---------------------------
A fatal error has occurred and debugging needs to be terminated. For more details, please see the Microsoft Help and Support web site. HRESULT=0x80070057. ErrorCode=0x0.
---------------------------
OK   
---------------------------

GlobalCache name works fine though

[Barsonax]

Ah I see, jup those constructors have to be made public

Strange that the name GlobalResolver crashes visual studio, maybe its some kind of reserved name?

[TETYYS]

Strange that the name GlobalResolver crashes visual studio, maybe its some kind of reserved name?

Error message is very abstract so I can't really tell. This problem is tracked here:

https://developercommunity.visualstudio.com/content/problem/934961/this-specific-variable-naming-crashes-visual-studi.html

[Barsonax]

Released a prerelease package: https://www.nuget.org/packages/Singularity/0.17.3-addcachecleanup0003

The constructors are now public, additionally I added a Remove method to the constructor resolver cache.

Still have to do more testing though but it seems this case is working now if you instantiate the resolver yourself.

[Barsonax]

Decided to disable the caching by default. If you want to do it you can do so explicitly by creating the resolver yourself and wrapping it. This way it should be more clear when instances might be kept alive.

Caching only really makes sense anyway when you keep registering the same services multiple times.