dotnet / runtime

.NET is a cross-platform runtime for cloud, mobile, desktop, and IoT apps.
https://docs.microsoft.com/dotnet/core/
MIT License
15.29k stars 4.74k forks source link

AssemblyLoadContext assemblies are not garbage collected #32646

Open BineG opened 4 years ago

BineG commented 4 years ago

I'm trying to implement a kind of a cached assembly execution, where if a certain context exists it is executed or updated, and if a new version is uploaded, it must be disposed before recreating it.

I've boiled the code down to this example below, but it seems the context is not being cleared, since the memory consumption is always increasing. I'm struggling to find anything which could be keeping the context alive

class Program
{
    static void Main(string[] args)
    {
        AssemblyExecutor executor = new AssemblyExecutor();

        for (int i = 0; i < 1000; i++)
        {
            var assemblyPath = Path.Combine(Directory.GetCurrentDirectory(), "..", "..", "..", "..", "Assembly", "DotNetRuleTestImplementation.dll");
            executor.Execute(assemblyPath);

            Console.WriteLine($"Iteration {i} completed");

            if (i > 0 && i % 10 == 0)
            {
                executor.Clear();

                GC.Collect();
                GC.WaitForPendingFinalizers();

                Console.WriteLine("GC called");
            }
        }

        executor.Clear();

        while (true)
        {
            Thread.Sleep(100);

            GC.Collect();
            GC.WaitForPendingFinalizers();
        }
    }
}

public class AssemblyExecutor
{
    private readonly ConcurrentDictionary<string, CollectibleAssemblyLoadContext> references = new ConcurrentDictionary<string, CollectibleAssemblyLoadContext>();

    [MethodImpl(MethodImplOptions.NoInlining)]
    public void Clear()
    {
        var keys = references.Keys.ToList();

        List<WeakReference> rfs = new List<WeakReference>();
        foreach (var k in keys)
        {
            references.TryRemove(k, out CollectibleAssemblyLoadContext ctx);

            ctx.Unload();

            WeakReference wr = new WeakReference(ctx);
            //wr.Target = null;                

            rfs.Add(wr);
        }

        for (int i = 0; i < 20 && rfs.Any(r => r.IsAlive); i++)
        {
            GC.Collect();
            GC.WaitForPendingFinalizers();

            Thread.Sleep(50);
        }
    }

    [MethodImpl(MethodImplOptions.NoInlining)]
    public void Execute(string assemblyPath)
    {
        var wr = references.AddOrUpdate(
                    assemblyPath,
                    (k) =>
                    {
                        return CreateReference(assemblyPath);
                    },
                    (k, existingReference) =>
                    {
                        if (existingReference == null || !existingReference.Assemblies.Any())
                        {
                            return CreateReference(assemblyPath);
                        }

                        return existingReference;
                    }
                );

        ExecuteAssembly(wr);
    }

    [MethodImpl(MethodImplOptions.NoInlining)]
    private void ExecuteAssembly(CollectibleAssemblyLoadContext context)
    {
        if (context == null)
            return;

        var assembly = context.Assemblies.FirstOrDefault(a => a.ExportedTypes.Any(t => t.Name == "TestEntry"));

        var types = assembly.GetTypes().ToList();

        var type = assembly.GetType("DotNetRuleTestImplementation.TestEntry");

        var greetMethod = type.GetMethod("Execute");

        var instance = Activator.CreateInstance(type);
        var result = greetMethod.Invoke(instance, new object[] { null, null });
    }

    private CollectibleAssemblyLoadContext CreateReference(string assemblyPath)
    {
        Console.WriteLine($"Creating new context");
        var context = new CollectibleAssemblyLoadContext(assemblyPath);
        using (var fs = new FileStream(assemblyPath, FileMode.Open, FileAccess.Read))
        {
            var assembly = context.LoadFromStream(fs);
        }

        return context;
    }
}

public class CollectibleAssemblyLoadContext : AssemblyLoadContext
{
    private AssemblyDependencyResolver _resolver;

    public CollectibleAssemblyLoadContext(string path)
        : base(isCollectible: true)
    {
        _resolver = new AssemblyDependencyResolver(path);
    }

    protected override Assembly Load(AssemblyName assemblyName)
    {
        var deps = AssemblyLoadContext.Default;
        var res = deps.Assemblies.Where(d => d.FullName.Contains(assemblyName.Name)).ToList();
        if (res.Any())
        {
            return Assembly.Load(new AssemblyName(res.First().FullName));
        }

        string assemblyPath = _resolver.ResolveAssemblyToPath(assemblyName);
        if (assemblyPath == null)
            return null;

        return LoadFromAssemblyPath(assemblyPath);
    }
}
vitek-karas commented 4 years ago

/cc @janvorli

What does the assembly being loaded into collectible load context do? Note that there are certain APIs in framework (and some NuGet packages) which will cache information which may end up holding direct references to the assembly, making it impossible to unload.

BineG commented 4 years ago

This specific one not much. It send a couple of records into Kafka and ElasticSearch via clients shared in the Default AssemblyContext.

I did figure out that if I change the ALC's Load method from

protected override Assembly Load(AssemblyName assemblyName)
{
    var deps = AssemblyLoadContext.Default;
    var res = deps.Assemblies.Where(d => d.FullName.Contains(assemblyName.Name)).ToList();
    if (res.Any())
    {
        return Assembly.Load(new AssemblyName(res.First().FullName));
    }

    string assemblyPath = _resolver.ResolveAssemblyToPath(assemblyName);
    if (assemblyPath == null)
        return null;

    return LoadFromAssemblyPath(assemblyPath);
}

to

    protected override Assembly Load(AssemblyName assemblyName)
    {
        var deps = AssemblyLoadContext.Default;
        var res = deps.Assemblies.Where(d => d.FullName.Contains(assemblyName.Name)).ToList();
        if (res.Any())
        {
            return null;
        }

        string assemblyPath = _resolver.ResolveAssemblyToPath(assemblyName);
        if (assemblyPath == null)
            return null;

        return LoadFromAssemblyPath(assemblyPath);
    }

So basically if I return null when a wanted assembly is in a Default ACL the memory consumption doesn't seem to be rising all the time although in the check loop the WeakReference is still always alive.

slika

Unfortunately in that case I also get a Fatal error now while debugging so I cant't really analyse it which is not ideal..

slika

janvorli commented 4 years ago

@BineG the best way to figure out what's holding the assemblies alive is to use WinDbg, as I've described in the https://docs.microsoft.com/en-us/dotnet/standard/assembly/unloadability#troubleshoot-unloadability-issues. With your sample, you can run it under WinDbg and wait until it gets into that infinite loop at the end of Main. Then break the execution and follow the steps described in the document.

janvorli commented 4 years ago

Btw, the VS crash when debugging unloadable code is tracked by https://github.com/dotnet/runtime/issues/2317, it was already fixed in master and the PR https://github.com/dotnet/coreclr/pull/28023 is porting that fix to 3.1.

Edit: Fixed the link to the porting PR

BineG commented 4 years ago

@janvorli Ok thank you for your reply. When can we expect the PR to be merged?

I already tried using WinDbg but without much success since I'm unfamiliar with that tool. I will try again and post the findings here.

janvorli commented 4 years ago

When can we expect the PR to be merged?

Based on the comment in the porting PR, it was approved for April release.

agocke commented 4 years ago

Moving this one out until we've identified the root cause