Integrate compiled factory-generator caches into ReflectionCacheSet infrastructure (#1461)

The two compiled factory-delegate generator caches (previously raw static
ConcurrentDictionary fields on FactoryGenerator) now live as named entries in
ReflectionCacheSet.Shared.Internal via a new ReflectionCacheTypeKeyedDictionary<TDiscriminator,TValue>
type, so they participate in ReflectionCacheSet.Clear() / Clear(predicate) calls. This prevents
Type references from collectible AssemblyLoadContexts being pinned forever by these caches,
fixing the memory-leak scenario described in #1461.

Author: tillig

src/Autofac/Core/InternalReflectionCaches.cs

@@ -3,6 +3,7 @@
 
 using System.Reflection;
 using Autofac.Core.Activators.Reflection;
+using Autofac.Features.GeneratedFactories;
 using Autofac.Util;
 using Autofac.Util.Cache;
 
@@ -48,6 +49,20 @@ public InternalReflectionCaches(ReflectionCacheSet set)
         ServiceKeyParameterAttributes = set.GetOrCreateCache<ReflectionCacheParameterDictionary<bool>>(nameof(ServiceKeyParameterAttributes));
         ServiceKeyPropertyAttributes = set.GetOrCreateCache<ReflectionCacheDictionary<PropertyInfo, bool>>(nameof(ServiceKeyPropertyAttributes));
         ServiceKeyUsageByType = set.GetOrCreateCache<ReflectionCacheDictionary<Type, bool>>(nameof(ServiceKeyUsageByType));
+
+        GeneratedFactoryServiceOnlyGenerators = set.GetOrCreateCache(
+            nameof(GeneratedFactoryServiceOnlyGenerators),
+            _ => new ReflectionCacheTypeKeyedDictionary<ParameterMapping, Func<Service, IComponentContext, IEnumerable<Parameter>, Delegate>>
+            {
+                Usage = ReflectionCacheUsage.All,
+            });
+
+        GeneratedFactoryServiceRegistrationGenerators = set.GetOrCreateCache(
+            nameof(GeneratedFactoryServiceRegistrationGenerators),
+            _ => new ReflectionCacheTypeKeyedDictionary<ParameterMapping, Func<Service, ServiceRegistration, IComponentContext, IEnumerable<Parameter>, Delegate>>
+            {
+                Usage = ReflectionCacheUsage.All,
+            });
     }
 
     /// <summary>
@@ -179,4 +194,24 @@ public ReflectionCacheDictionary<Type, bool> ModuleOverridesAttachToRegistration
     {
         get;
     }
+
+    /// <summary>
+    /// Gets the cache of compiled factory-delegate generators for
+    /// <see cref="Features.GeneratedFactories.FactoryGenerator"/> instances that resolve via
+    /// <c>ResolveService</c>. Keyed on <c>(delegateType, effectiveParameterMapping)</c>.
+    /// </summary>
+    public ReflectionCacheTypeKeyedDictionary<ParameterMapping, Func<Service, IComponentContext, IEnumerable<Parameter>, Delegate>> GeneratedFactoryServiceOnlyGenerators
+    {
+        get;
+    }
+
+    /// <summary>
+    /// Gets the cache of compiled factory-delegate generators for
+    /// <see cref="Features.GeneratedFactories.FactoryGenerator"/> instances that resolve via
+    /// <see cref="IComponentContext.ResolveComponent"/>. Keyed on <c>(delegateType, effectiveParameterMapping)</c>.
+    /// </summary>
+    public ReflectionCacheTypeKeyedDictionary<ParameterMapping, Func<Service, ServiceRegistration, IComponentContext, IEnumerable<Parameter>, Delegate>> GeneratedFactoryServiceRegistrationGenerators
+    {
+        get;
+    }
 }

src/Autofac/Features/GeneratedFactories/FactoryGenerator.cs

@@ -1,7 +1,6 @@
 // Copyright (c) Autofac Project. All rights reserved.
 // Licensed under the MIT License. See LICENSE in the project root for license information.
 
-using System.Collections.Concurrent;
 using System.Globalization;
 using System.Linq.Expressions;
 using System.Reflection;
@@ -16,22 +15,6 @@ namespace Autofac.Features.GeneratedFactories;
 /// </summary>
 public class FactoryGenerator
 {
-    /// <summary>
-    /// Cached compiled generators for the <see cref="FactoryGenerator(Type, Service, ParameterMapping)"/> overload
-    /// (resolves via <c>ResolveService</c>). Keyed on <c>(delegateType, effectiveParameterMapping)</c>.
-    /// Exposed internally for test observability only.
-    /// </summary>
-    internal static readonly ConcurrentDictionary<(Type, ParameterMapping), Func<Service, IComponentContext, IEnumerable<Parameter>, Delegate>>
-        ServiceOnlyGeneratorCache = new();
-
-    /// <summary>
-    /// Cached compiled generators for the <see cref="FactoryGenerator(Type, Service, ServiceRegistration, ParameterMapping)"/> overload
-    /// (resolves via <see cref="IComponentContext.ResolveComponent"/>). Keyed on <c>(delegateType, effectiveParameterMapping)</c>.
-    /// Exposed internally for test observability only.
-    /// </summary>
-    internal static readonly ConcurrentDictionary<(Type, ParameterMapping), Func<Service, ServiceRegistration, IComponentContext, IEnumerable<Parameter>, Delegate>>
-        ServiceRegistrationGeneratorCache = new();
-
     // The explicit '!' default is ok because the code is never executed, it's just used by
     // the expression tree.
     private static readonly ConstructorInfo _requestConstructor
@@ -61,7 +44,9 @@ public FactoryGenerator(Type delegateType, Service service, ParameterMapping par
         // or compile it once on first use. The cached delegate is parameterised on 'service'
         // (passed at invocation time) so it contains no instance-specific captures and is safe
         // to share across all FactoryGenerator instances with the same structural signature.
-        var compiledGenerator = ServiceOnlyGeneratorCache.GetOrAdd(
+        // Accessing ReflectionCacheSet.Shared at point-of-use (not stored in a field) ensures
+        // correct weak-reference collection of the cache set.
+        var compiledGenerator = ReflectionCacheSet.Shared.Internal.GeneratedFactoryServiceOnlyGenerators.GetOrAdd(
             (delegateType, pm),
             static key => CreateServiceOnlyGenerator(key.Item1, key.Item2));
 
@@ -88,7 +73,9 @@ public FactoryGenerator(Type delegateType, Service service, ServiceRegistration
         // or compile it once on first use. The cached delegate is parameterised on both 'service'
         // and 'productRegistration' (passed at invocation time), so it contains no instance-specific
         // captures and is safe to share across all FactoryGenerator instances for the same delegate type.
-        var compiledGenerator = ServiceRegistrationGeneratorCache.GetOrAdd(
+        // Accessing ReflectionCacheSet.Shared at point-of-use (not stored in a field) ensures
+        // correct weak-reference collection of the cache set.
+        var compiledGenerator = ReflectionCacheSet.Shared.Internal.GeneratedFactoryServiceRegistrationGenerators.GetOrAdd(
             (delegateType, pm),
             static key => CreateServiceRegistrationGenerator(key.Item1, key.Item2));
 

src/Autofac/Util/Cache/ReflectionCacheTypeKeyedDictionary.cs

@@ -0,0 +1,49 @@
+// Copyright (c) Autofac Project. All rights reserved.
+// Licensed under the MIT License. See LICENSE in the project root for license information.
+
+using System.Collections.Concurrent;
+using System.Reflection;
+using Autofac.Core;
+
+namespace Autofac.Util.Cache;
+
+/// <summary>
+/// A reflection cache dictionary, keyed on a <c>(<see cref="Type"/>, <typeparamref name="TDiscriminator"/>)</c>
+/// tuple where <typeparamref name="TDiscriminator"/> is a non-assembly-bearing discriminator (e.g. an enum).
+/// Only the <see cref="Type"/> component of the key participates in assembly-predicate–based clearing.
+/// </summary>
+/// <typeparam name="TDiscriminator">
+/// The second component of the tuple key; must be non-null but need not be a <see cref="MemberInfo"/>.
+/// </typeparam>
+/// <typeparam name="TValue">The value type.</typeparam>
+internal sealed class ReflectionCacheTypeKeyedDictionary<TDiscriminator, TValue>
+    : ConcurrentDictionary<(Type, TDiscriminator), TValue>, IReflectionCache
+    where TDiscriminator : notnull
+{
+    /// <inheritdoc />
+    public ReflectionCacheUsage Usage { get; set; } = ReflectionCacheUsage.All;
+
+    /// <inheritdoc />
+    public void Clear(ReflectionCacheClearPredicate predicate)
+    {
+        if (predicate is null)
+        {
+            throw new ArgumentNullException(nameof(predicate));
+        }
+
+        if (Count == 0)
+        {
+            return;
+        }
+
+        var reusableAssemblySet = new HashSet<Assembly>();
+
+        foreach (var kvp in this)
+        {
+            if (predicate(kvp.Key.Item1, TypeAssemblyReferenceProvider.GetAllReferencedAssemblies(kvp.Key.Item1, reusableAssemblySet)))
+            {
+                TryRemove(kvp.Key, out _);
+            }
+        }
+    }
+}

test/Autofac.Test/Features/GeneratedFactories/GeneratedFactoriesTests.cs

@@ -476,9 +476,9 @@ public void FactoryDelegate_CompiledGeneratorCached_SiblingScopes()
         scope1.Resolve<FuncConsumer1461>();
 
         // After the first resolve, the compiled generator must be present in the cache.
-        Assert.True(Autofac.Features.GeneratedFactories.FactoryGenerator.ServiceRegistrationGeneratorCache.ContainsKey(cacheKey));
+        Assert.True(ReflectionCacheSet.Shared.Internal.GeneratedFactoryServiceRegistrationGenerators.ContainsKey(cacheKey));
 
-        var countAfterFirstScope = Autofac.Features.GeneratedFactories.FactoryGenerator.ServiceRegistrationGeneratorCache.Count;
+        var countAfterFirstScope = ReflectionCacheSet.Shared.Internal.GeneratedFactoryServiceRegistrationGenerators.Count;
 
         using var scope2 = container.BeginLifetimeScope(c => c.RegisterType<FuncConsumer1461>());
         scope2.Resolve<FuncConsumer1461>();
@@ -488,7 +488,7 @@ public void FactoryDelegate_CompiledGeneratorCached_SiblingScopes()
 
         // Resolving the same Func<T> from additional sibling scopes must not add new cache entries;
         // the count must remain stable after the first scope warms the entry.
-        var countAfterAllScopes = Autofac.Features.GeneratedFactories.FactoryGenerator.ServiceRegistrationGeneratorCache.Count;
+        var countAfterAllScopes = ReflectionCacheSet.Shared.Internal.GeneratedFactoryServiceRegistrationGenerators.Count;
         Assert.Equal(countAfterFirstScope, countAfterAllScopes);
     }
 
@@ -638,4 +638,25 @@ public void FactoryDelegate_RespectsChildScopeContext_SiblingScopes()
         Assert.Same(result1, f1());
         Assert.Same(result2, f2());
     }
+
+    [Fact]
+    public void FactoryDelegate_CompiledGeneratorCache_ClearedByReflectionCacheSetClear()
+    {
+        // #1461: The generated-factory caches must participate in ReflectionCacheSet.Clear()
+        // so that types from collectible AssemblyLoadContexts can be fully unloaded.
+        var builder = new ContainerBuilder();
+        builder.RegisterType<Dependency1461>();
+        using var container = builder.Build();
+
+        using var scope = container.BeginLifetimeScope(c => c.RegisterType<FuncConsumer1461>());
+        scope.Resolve<FuncConsumer1461>();
+
+        var serviceRegCache = ReflectionCacheSet.Shared.Internal.GeneratedFactoryServiceRegistrationGenerators;
+
+        Assert.NotEmpty(serviceRegCache);
+
+        ReflectionCacheSet.Shared.Clear();
+
+        Assert.Empty(serviceRegCache);
+    }
 }