Make ILazyLoader not IDisposable (#32345) (#32463)

Fixes #32267

The problem here is that ILazyLoader is a transient IDisposable service, which means that the service scope will keep track of instances created in the scope. However, when using context pooling, the service scope is not disposed because it is instead re-used. This means that the scope keeps getting more and more instances added, and never clears them out.

The fix is to make the service not IDisposable. Instead, we create instances from our own internal factory where we keep track of the instances created. These can then be disposed and freed when the context is places back in the pool, or when the scope is disposed thus disposing the factory.

Author: ajcvickers

src/EFCore.Abstractions/Infrastructure/ILazyLoader.cs

@@ -20,7 +20,7 @@ namespace Microsoft.EntityFrameworkCore.Infrastructure;
 ///         See <see href="https://aka.ms/efcore-docs-lazy-loading">Lazy loading</see> for more information and examples.
 ///     </para>
 /// </remarks>
-public interface ILazyLoader : IDisposable
+public interface ILazyLoader
 {
     /// <summary>
     ///     Sets the given navigation as known to be completely loaded or known to be
@@ -66,4 +66,9 @@ Task LoadAsync(
         object entity,
         CancellationToken cancellationToken = default,
         [CallerMemberName] string navigationName = "");
+
+    /// <summary>
+    ///     Disposes the loader.
+    /// </summary>
+    void Dispose();
 }

src/EFCore/ChangeTracking/Internal/StateManager.cs

@@ -695,6 +695,11 @@ public virtual void Unsubscribe(bool resetting)
                     {
                         disposable.Dispose();
                     }
+                    else if (resetting
+                             && service is ILazyLoader lazyLoader)
+                    {
+                        lazyLoader.Dispose();
+                    }
                     else if (service is not IInjectableService detachable
                              || detachable.Detaching(Context, entry.Entity))
                     {

src/EFCore/Infrastructure/EntityFrameworkServicesBuilder.cs

@@ -126,6 +126,7 @@ public static readonly IDictionary<Type, ServiceCharacteristics> CoreServices
             { typeof(IDbContextLogger), new ServiceCharacteristics(ServiceLifetime.Scoped) },
             { typeof(IAdHocMapper), new ServiceCharacteristics(ServiceLifetime.Scoped) },
             { typeof(ILazyLoader), new ServiceCharacteristics(ServiceLifetime.Transient) },
+            { typeof(ILazyLoaderFactory), new ServiceCharacteristics(ServiceLifetime.Scoped) },
             { typeof(IParameterBindingFactory), new ServiceCharacteristics(ServiceLifetime.Singleton, multipleRegistrations: true) },
             { typeof(ITypeMappingSourcePlugin), new ServiceCharacteristics(ServiceLifetime.Singleton, multipleRegistrations: true) },
             {
@@ -285,12 +286,14 @@ public virtual EntityFrameworkServicesBuilder TryAddCoreServices()
         TryAdd<IDesignTimeModel>(p => new DesignTimeModel(GetContextServices(p)));
         TryAdd(p => GetContextServices(p).CurrentContext);
         TryAdd<IDbContextOptions>(p => GetContextServices(p).ContextOptions);
+        TryAdd<IResettableService, ILazyLoaderFactory>(p => p.GetRequiredService<ILazyLoaderFactory>());
         TryAdd<IResettableService, IStateManager>(p => p.GetRequiredService<IStateManager>());
         TryAdd<IResettableService, IDbContextTransactionManager>(p => p.GetRequiredService<IDbContextTransactionManager>());
         TryAdd<IEvaluatableExpressionFilter, EvaluatableExpressionFilter>();
         TryAdd<IValueConverterSelector, ValueConverterSelector>();
         TryAdd<IConstructorBindingFactory, ConstructorBindingFactory>();
-        TryAdd<ILazyLoader, LazyLoader>();
+        TryAdd<ILazyLoaderFactory, LazyLoaderFactory>();
+        TryAdd<ILazyLoader>(p => p.GetRequiredService<ILazyLoaderFactory>().Create());
         TryAdd<IParameterBindingFactories, ParameterBindingFactories>();
         TryAdd<IMemberClassifier, MemberClassifier>();
         TryAdd<IPropertyParameterBindingFactory, PropertyParameterBindingFactory>();

src/EFCore/Infrastructure/Internal/ILazyLoaderFactory.cs

@@ -0,0 +1,21 @@
+// Licensed to the .NET Foundation under one or more agreements.
+// The .NET Foundation licenses this file to you under the MIT license.
+
+namespace Microsoft.EntityFrameworkCore.Infrastructure.Internal;
+
+/// <summary>
+///     This is an internal API that supports the Entity Framework Core infrastructure and not subject to
+///     the same compatibility standards as public APIs. It may be changed or removed without notice in
+///     any release. You should only use it directly in your code with extreme caution and knowing that
+///     doing so can result in application failures when updating to a new Entity Framework Core release.
+/// </summary>
+public interface ILazyLoaderFactory : IDisposable, IResettableService
+{
+    /// <summary>
+    ///     This is an internal API that supports the Entity Framework Core infrastructure and not subject to
+    ///     the same compatibility standards as public APIs. It may be changed or removed without notice in
+    ///     any release. You should only use it directly in your code with extreme caution and knowing that
+    ///     doing so can result in application failures when updating to a new Entity Framework Core release.
+    /// </summary>
+    ILazyLoader Create();
+}

src/EFCore/Infrastructure/Internal/LazyLoaderFactory.cs

@@ -0,0 +1,81 @@
+// Licensed to the .NET Foundation under one or more agreements.
+// The .NET Foundation licenses this file to you under the MIT license.
+
+namespace Microsoft.EntityFrameworkCore.Infrastructure.Internal;
+
+/// <summary>
+///     This is an internal API that supports the Entity Framework Core infrastructure and not subject to
+///     the same compatibility standards as public APIs. It may be changed or removed without notice in
+///     any release. You should only use it directly in your code with extreme caution and knowing that
+///     doing so can result in application failures when updating to a new Entity Framework Core release.
+/// </summary>
+public class LazyLoaderFactory : ILazyLoaderFactory
+{
+    private readonly ICurrentDbContext _currentContext;
+    private readonly IDiagnosticsLogger<DbLoggerCategory.Infrastructure> _logger;
+    private readonly List<ILazyLoader> _loaders = new();
+
+    /// <summary>
+    ///     This is an internal API that supports the Entity Framework Core infrastructure and not subject to
+    ///     the same compatibility standards as public APIs. It may be changed or removed without notice in
+    ///     any release. You should only use it directly in your code with extreme caution and knowing that
+    ///     doing so can result in application failures when updating to a new Entity Framework Core release.
+    /// </summary>
+    public LazyLoaderFactory(
+        ICurrentDbContext currentContext,
+        IDiagnosticsLogger<DbLoggerCategory.Infrastructure> logger)
+    {
+        _currentContext = currentContext;
+        _logger = logger;
+    }
+
+    /// <summary>
+    ///     This is an internal API that supports the Entity Framework Core infrastructure and not subject to
+    ///     the same compatibility standards as public APIs. It may be changed or removed without notice in
+    ///     any release. You should only use it directly in your code with extreme caution and knowing that
+    ///     doing so can result in application failures when updating to a new Entity Framework Core release.
+    /// </summary>
+    public virtual ILazyLoader Create()
+    {
+        var loader = new LazyLoader(_currentContext, _logger);
+        _loaders.Add(loader);
+        return loader;
+    }
+
+    /// <summary>
+    ///     This is an internal API that supports the Entity Framework Core infrastructure and not subject to
+    ///     the same compatibility standards as public APIs. It may be changed or removed without notice in
+    ///     any release. You should only use it directly in your code with extreme caution and knowing that
+    ///     doing so can result in application failures when updating to a new Entity Framework Core release.
+    /// </summary>
+    public void Dispose()
+    {
+        foreach (var loader in _loaders)
+        {
+            loader.Dispose();
+        }
+        _loaders.Clear();
+    }
+
+    /// <summary>
+    ///     This is an internal API that supports the Entity Framework Core infrastructure and not subject to
+    ///     the same compatibility standards as public APIs. It may be changed or removed without notice in
+    ///     any release. You should only use it directly in your code with extreme caution and knowing that
+    ///     doing so can result in application failures when updating to a new Entity Framework Core release.
+    /// </summary>
+    public void ResetState()
+        => Dispose();
+
+    /// <summary>
+    ///     This is an internal API that supports the Entity Framework Core infrastructure and not subject to
+    ///     the same compatibility standards as public APIs. It may be changed or removed without notice in
+    ///     any release. You should only use it directly in your code with extreme caution and knowing that
+    ///     doing so can result in application failures when updating to a new Entity Framework Core release.
+    /// </summary>
+    public Task ResetStateAsync(CancellationToken cancellationToken = default)
+    {
+        Dispose();
+
+        return Task.CompletedTask;
+    }
+}

test/EFCore.Tests/Infrastructure/EntityFrameworkServicesBuilderTest.cs

@@ -266,7 +266,7 @@ private static void TestMultipleScoped(Action<EntityFrameworkServicesBuilder> tr
         {
             services = context.GetService<IEnumerable<IResettableService>>().ToList();
 
-            Assert.Equal(3, services.Count);
+            Assert.Equal(4, services.Count);
             Assert.Contains(typeof(FakeResetableService), services.Select(s => s.GetType()));
             Assert.Contains(typeof(StateManager), services.Select(s => s.GetType()));
             Assert.Contains(typeof(InMemoryTransactionManager), services.Select(s => s.GetType()));
@@ -281,7 +281,7 @@ private static void TestMultipleScoped(Action<EntityFrameworkServicesBuilder> tr
         {
             var newServices = context.GetService<IEnumerable<IResettableService>>().ToList();
 
-            Assert.Equal(3, newServices.Count);
+            Assert.Equal(4, newServices.Count);
 
             foreach (var service in newServices)
             {