Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

DB Context pooling with proxies resulting in memory issues in 8.0 RC2 #32267

Closed
Ottodeklerk opened this issue Nov 9, 2023 · 6 comments · Fixed by #32345 or #32463
Closed

DB Context pooling with proxies resulting in memory issues in 8.0 RC2 #32267

Ottodeklerk opened this issue Nov 9, 2023 · 6 comments · Fixed by #32345 or #32463
Assignees
Labels
area-proxies breaking-change closed-fixed The issue has been fixed and is/will be included in the release indicated by the issue milestone. customer-reported Servicing-approved type-bug
Milestone

Comments

@Ottodeklerk
Copy link

Ottodeklerk commented Nov 9, 2023

DB Context pooling with proxies enabled

Issue #25486 describes that this issue should be resolved in .NET 8 RC1. When comparing .NET7 with .NET8 RC2 no differences in memory handling are noticable.
When running the code below the process quickly passes the 1GB memory mark. When disabling proxies or using adddbcontext instead of pooling the memory usage is more consistent (as it should be after disposing the scope, I assume).

Code example

var testProcess = new TestSetup();
testProcess.InitializeServiceProvider();
await testProcess.QueryTest();

public class TestSetup
{
    private IServiceProvider _serviceProvider;
    public IServiceProvider InitializeServiceProvider()
    {
        var sc = new ServiceCollection();
        sc.AddDbContextPool<Context>(options =>
        {
            options.UseInMemoryDatabase("test").UseLazyLoadingProxies();
        });

        // For comparison (does not result in high memory usage)
        //sc.AddDbContext<Context>(options =>
        //{
        //    options.UseInMemoryDatabase("test").UseLazyLoadingProxies();
        //});

        _serviceProvider = sc.BuildServiceProvider();
        return _serviceProvider;
    }

    public async Task QueryTest()
    {
        await using ( var insertScope = _serviceProvider.CreateAsyncScope())
        {
            var insertScopeSp = insertScope.ServiceProvider;
            var insertScopeCt = insertScopeSp.GetRequiredService<Context>();
            for (var x = 1; x <= 10000; x++)
                insertScopeCt.TestModels.Add(new TestModel() {Id = x, Test = "test"});
            await insertScopeCt.SaveChangesAsync();
        }

        var i = 0;
        while (i < 100000)
        {
            i++;
            await using var scope = _serviceProvider.CreateAsyncScope();
            var sp = scope.ServiceProvider;
            var contextInstance = sp.GetRequiredService<Context>();
            var list = await contextInstance.TestModels.ToListAsync();
        }

        Console.ReadLine();

    }


    // DB Context
    public class TestModel
    {
        public long Id { get; set; }
        public string Test { get; set; }
    }
    public class Context : DbContext
    {
        public Context(DbContextOptions options) : base(options) { }
        public DbSet<TestModel> TestModels { get; set; }
    }
}

Memory snapshot comparison with Pooling only
image

image

Memory snapshot comparison with Pooling and proxies enabled
image

image

EF Core version:

    <PackageReference Include="Microsoft.EntityFrameworkCore" Version="8.0.0-rc.2.23480.1" />
    <PackageReference Include="Microsoft.EntityFrameworkCore.InMemory" Version="8.0.0-rc.2.23480.1" />
    <PackageReference Include="Microsoft.EntityFrameworkCore.Proxies" Version="8.0.0-rc.2.23480.1" />
    <PackageReference Include="Microsoft.EntityFrameworkCore.Tools" Version="8.0.0-rc.2.23480.1">

Database provider: InMemory
Target framework: 8.0
Operating system: Win-11
IDE: VS 2022 Preview 17.8.0 Preview 3

@roji
Copy link
Member

roji commented Nov 12, 2023

@Ottodeklerk is this something you've seen in a real database provider as well, e.g. SQL Server or SQLite?

@Ottodeklerk
Copy link
Author

Ottodeklerk commented Nov 12, 2023

I just tried the example above with the SQL server provider (8.0.0-rc.2.23480.1), and the results look the same.

Pooling and proxies enabled:
image

Pooling without proxies:
image

The issue came to our attention since our .NET 7 (SQL server based) production environment has some serious memory issues and occasional hangs when the GC is active. A lot of the proxy related objects (LazyLoader) seem to be stuck in gen2.

For now, we will disable pooling as a quick workaround since it creates more performance issues than it should resolve.

If you need anything else please let me know.

@roji
Copy link
Member

roji commented Nov 12, 2023

/cc @ajcvickers

@Ottodeklerk
Copy link
Author

In case it helps, the following (random) cases were tested to see if it makes any difference:

  • Changing the pool size.
  • Using sync/async scopes.
  • Using buffering/streaming (ToListAsync vs AsAsyncEnumerable ).
  • Using .AsNoTracking.
  • Disabling LazyLoadingEnabled on the ChangeTracker (before the query).
  • Calling SaveChanges after the read-only tolist (to possibly reset the state internally?).
  • Setting entity state detached manually using the EntityEntry.
  • Calling clear or ResetStateAsync on the StateManager manually before leaving the scope. Example:
    image

Unfortunately none of the above changed the outcome.

@ajcvickers
Copy link
Member

Note for triage: we null out the DbContext, but the Castle interceptor still has a reference to the entity type:

image

@Ottodeklerk
Copy link
Author

Clearing the LazyLoader disposables in the DbContext service scope seems to keep the memory usage consistent and does not break pooling functionality.
It's more of a hacky workaround, but maybe it helps to find the root of the issue.

private bool DisposeSync(bool leaseActive, bool contextShouldBeDisposed)
{

    // Dispose LazyLoader objects in the Disposables list without disposing all services required by the pooled context.
    var props = _serviceScope?.GetType().GetProperties(BindingFlags.Instance | BindingFlags.NonPublic)
        .First(e => e.Name == "Disposables");
    var toDispose = (props!.GetValue(_serviceScope) as List<object>);
    var toDisposeCopy = toDispose!.ToList();

    for (var i = toDisposeCopy.Count - 1; i >= 0; i--)
    {
        if (toDisposeCopy[i] is LazyLoader lazyLoader)
        {
            lazyLoader.Dispose();
            toDispose!.Remove(lazyLoader);
        }
    }

    if (leaseActive)
    {
        if (contextShouldBeDisposed)
        {
            _disposed = true;
            _lease = DbContextLease.InactiveLease;
        }
    }
    else if (!_disposed)
    {
        EntityFrameworkEventSource.Log.DbContextDisposing();

        _dbContextDependencies?.InfrastructureLogger.ContextDisposed(this);

        _disposed = true;

        _dbContextDependencies?.StateManager.Unsubscribe(resetting: true);

        _dbContextDependencies = null;
        _changeTracker = null;
        _database = null;
        _configurationSnapshot = null;

        SavingChanges = null;
        SavedChanges = null;
        SaveChangesFailed = null;

        return true;
    }

    return false;
}

image

@ajcvickers ajcvickers self-assigned this Nov 17, 2023
@ajcvickers ajcvickers added this to the 8.0.x milestone Nov 17, 2023
ajcvickers added a commit that referenced this issue Nov 18, 2023
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.
ajcvickers added a commit that referenced this issue Nov 20, 2023
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.
@ajcvickers ajcvickers reopened this Nov 20, 2023
@ajcvickers ajcvickers removed this from the 8.0.x milestone Nov 20, 2023
@ajcvickers ajcvickers added this to the 8.0.x milestone Nov 30, 2023
ajcvickers added a commit that referenced this issue Nov 30, 2023
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.
@ajcvickers ajcvickers modified the milestones: 8.0.x, 8.0.2 Dec 1, 2023
ajcvickers added a commit that referenced this issue Dec 1, 2023
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.
@ajcvickers ajcvickers added the closed-fixed The issue has been fixed and is/will be included in the release indicated by the issue milestone. label Dec 5, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-proxies breaking-change closed-fixed The issue has been fixed and is/will be included in the release indicated by the issue milestone. customer-reported Servicing-approved type-bug
Projects
None yet
3 participants