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

Proxies retain a reference to pooled DbContext after it is returned to the pool #25486

Closed
rexxmagnus opened this issue Aug 11, 2021 · 11 comments · Fixed by #29929
Closed

Proxies retain a reference to pooled DbContext after it is returned to the pool #25486

rexxmagnus opened this issue Aug 11, 2021 · 11 comments · Fixed by #29929
Labels
area-dbcontext closed-fixed The issue has been fixed and is/will be included in the release indicated by the issue milestone. customer-reported punted-for-6.0 punted-for-7.0 Originally planned for the EF Core 7.0 (EF7) release, but moved out due to resource constraints. type-bug
Milestone

Comments

@rexxmagnus
Copy link

I've been using AddDbContextPool to inject an instance of my database into asp.net core controllers. In one instance, I also have a background task that runs via

services.AddHostedService<AppCode.SignalR.Daemon>();`

The Daemon implements IHostedService and IDisposable and retrieves IServiceScopeFactory in its constructor. A timer uses this to create a database context (with the scope correctly wrapped in a using statement) if it needs to update the database (it's monitoring which users are logged in and updating their session times if they log out). Through the day, the memory usage of the app slowly increases, butforcing periodic garbage collection helps to slow this a little. However, taking a memory dump shows that a lot of memory is being used by EntityFrameworkCore.Internal.LazyLoader at the very top of the flame graph.

RD28187824B03E_w3wp_29304 flameGraph1

Two entries below (on the line below LIB <<System.Private.CoreLib!List>> is a reference to DependencyInjection.ServiceLookup.ServiceProviderEngineScope and then further references to the database context pool (I'm using AddDbContextPool to inject the database)

The timer doesn't capture the database scope other than via ServiceProvider.GetRequiredService() so I can't see where this memory is being held onto, unless there's an issue with the lazyloader (although the code I'm accessing in the database doesn't reference any lazy loaded entities.

EF Core version: 5.0.8
Microsoft.EntityFrameworkCore.SqlServer
Target framework: NET 5.0
Operating system: Windows 10 (publishing to Azure web app)
IDE: Visual Studio 2019 16.9.5

@rexxmagnus
Copy link
Author

It's just occurred to me that when switching from AddDbContextPool to AddDbContext, I'm getting database disposed errors, caused by code I've put in an AuthorizationAttribute that accesses the database wrapped in a using statement. I've now fixed that by getting the scope via HttpContext.RequestServices.CreateScope(), so I'll see if it mitigates the issue.

@rexxmagnus
Copy link
Author

It appears that removing the using statement wrapping the datacontext in the AuthorizationAttribute and instead getting the scope from the httpcontext has stopped the issue. I can only suppose that objects were being left behind by the disposal of the context, when all other cases had it handled either by my web api's controllers via DI or the injection of the scope factory.

@ajcvickers ajcvickers added the closed-no-further-action The issue is closed and no further action is planned. label Aug 11, 2021
@rexxmagnus
Copy link
Author

It appears that removing the using statement wrapping the datacontext in the AuthorizationAttribute and instead getting the scope from the httpcontext has stopped the issue. I can only suppose that objects were being left behind by the disposal of the context, when all other cases had it handled either by my web api's controllers via DI or the injection of the scope factory.

@rexxmagnus rexxmagnus reopened this Aug 11, 2021
@rexxmagnus
Copy link
Author

rexxmagnus commented Aug 11, 2021

It seems I still had a small memory issue while using the DbContextPool, which has now reduced by switching to AddDbContext instead. I'm not sure whether this implies there is actually a memory leak in the lazyloading framework.
The flame graph is now much more even (the overall memory used by the entire hosting process is now approx 250mb, compared to 450ish from before), and the Daemon task doesn't feature in it.

RD28187824B03E_w3wp_34812 flameGraph1

@roji
Copy link
Member

roji commented Aug 11, 2021

@rexxmagnus we're going to have to see some code in order to investigate this, can you please submit a minimal repro project?

@rexxmagnus
Copy link
Author

rexxmagnus commented Aug 13, 2021

Hi, @roji
Here's a minimal project, whittled down from my production scenario. Unfortunately, I wasn't able to get the route for the api controller triggering correctly after I cut it back, but I'm sure you can work it out.
The issue appears to be with disposing a database instance when using AddDbContextPool (which I was doing unwittingly). Its disposal doesn't cause an error (but it does if using AddDbContext) and instead leads to a leak with the ef core lazyloading libraries.
I'm not sure whether my sample project would give a measurable leak, but I suspect it would get something from repeated hits on the test controller, since the timer daemon itself is handling the database correctly.


  <PropertyGroup>
    <TargetFramework>net5.0</TargetFramework>
    <AspNetCoreHostingModel>InProcess</AspNetCoreHostingModel>
    <CopyRefAssembliesToPublishDirectory>true</CopyRefAssembliesToPublishDirectory>
    <PlatformTarget>x64</PlatformTarget>
  </PropertyGroup>

 
  <PropertyGroup Condition="'$(Configuration)|$(Platform)'=='Debug|AnyCPU'">
   
  </PropertyGroup>

  <ItemGroup>
    <Content Remove="Web.config" />

    <PackageReference Include="Microsoft.AspNetCore.Authentication" Version="2.2.0" />
    <PackageReference Include="Microsoft.AspNetCore.Authentication.Cookies" Version="2.2.0" />
    <PackageReference Include="Microsoft.AspNetCore.Mvc.Core" Version="2.2.5" />
    <PackageReference Include="Microsoft.Data.SqlClient" Version="3.0.0" />
    <PackageReference Include="Microsoft.Extensions.Http" Version="5.0.0" />
    <PackageReference Include="Microsoft.EntityFrameworkCore.SqlServer" Version="5.0.9" />
    <PackageReference Include="System.Data.Common" Version="4.3.0" />

    <PackageReference Include="Microsoft.EntityFrameworkCore.Proxies" Version="5.0.9" />
    <PackageReference Include="Microsoft.Azure.Services.AppAuthentication" Version="1.6.1" />
    <PackageReference Include="Microsoft.Extensions.Logging.Console" Version="5.0.0" />

  </ItemGroup>

  <ItemGroup>
    <Folder Include="Properties\PublishProfiles\" />
    <Folder Include="Properties\ServiceDependencies\" />
  </ItemGroup>

</Project>
using Microsoft.AspNetCore.Builder;
using Microsoft.AspNetCore.Hosting;
using Microsoft.AspNetCore.Mvc;
using Microsoft.EntityFrameworkCore;
using Microsoft.Extensions.Configuration;
using Microsoft.Extensions.DependencyInjection;
using Microsoft.Extensions.Hosting;
using System;
using System.Collections.Generic;
using System.ComponentModel.DataAnnotations;
using System.ComponentModel.DataAnnotations.Schema;
using System.Linq;
using System.Text.Json.Serialization;
using System.Threading;
using System.Threading.Tasks;

namespace WebApp
{
    public class Program
    {
        public static void Main(string[] args)
        {
            CreateHostBuilder(args).Build().Run();
        }

        public static IHostBuilder CreateHostBuilder(string[] args) =>
        Host.CreateDefaultBuilder(args)
            .ConfigureWebHostDefaults(webBuilder =>
            {
                webBuilder.UseStartup<Startup>();
            }).ConfigureServices(services => {
                // Launch the daemon background thread after the website has started
                services.AddHostedService<Daemon>();
            });
    }


    public class Startup
    {

        public Startup(IConfiguration configuration, IWebHostEnvironment environment)
        {

            Configuration = configuration;
            HostingEnvironment = environment;

        }

        public IConfiguration Configuration { get; }
        public IWebHostEnvironment HostingEnvironment { get; }

        public void ConfigureServices(IServiceCollection services)
        {
            services.AddDbContextPool<MyDataContext>(options =>
            {
                options

                .UseLazyLoadingProxies()
                .UseSqlServer("myconnectionstring")
                .UseQueryTrackingBehavior(QueryTrackingBehavior.NoTracking);
            });

            services.AddAuthentication(Microsoft.AspNetCore.Authentication.Cookies.CookieAuthenticationDefaults.AuthenticationScheme).AddCookie();

            services.AddControllers().AddJsonOptions(options =>{

            });

            services.AddAuthorization();

            services.AddCors(options =>
            {
                options.AddPolicy("all",
                    builder =>
                    {
                        builder.AllowAnyOrigin().AllowAnyMethod().AllowAnyHeader();
                    });
            });

        }

        public void Configure(IApplicationBuilder app, IWebHostEnvironment env, IServiceProvider serviceProvider, IHostApplicationLifetime applicationLifetime)
        {

            app.UseDeveloperExceptionPage();

            app.UseRouting();

            app.UseCors("all");

            
            app.UseAuthentication();
            app.UseAuthorization();

            app.UseEndpoints(endpoints =>
            {
                endpoints.MapControllerRoute(name: "default", pattern: "{controller}/{action}");
            });
        }


    }


    public class MyDataContext : DbContext
    {
        public MyDataContext(DbContextOptions<MyDataContext> options) : base(options) { }

        public DbSet<User> Users { get; set; }
        public DbSet<UserExtended> UserExtended { get; set; }

    }

    [Table("User")]
    public class User
    {
        [Key, DatabaseGenerated(DatabaseGeneratedOption.None)]
        public Guid UserID { get; set; }

        // Other user properties
        public DateTime? LastLogin { get; set; }

        [InverseProperty("User")]
        public virtual ICollection<UserExtended> UserOutcodes { get; private set; }
    }

    [Table("UserExtended")]
    public class UserExtended
    {
        [Key, DatabaseGenerated(DatabaseGeneratedOption.None)]
        public Guid UserExtendedID { get; set; }

        [ForeignKey("User")]
        public Guid UserID { get; set; }

        // Other properties related to user


        // Foreign key
        public virtual User User { get; private set; }
    }


    internal class Daemon : IHostedService, IDisposable
    {

        private Timer _timer;

        private readonly IServiceScopeFactory _scopeFactory;

        public Daemon(IServiceScopeFactory scopeFactory)
        {
            _scopeFactory = scopeFactory;
        }

        public Task StartAsync(CancellationToken cancellationToken)
        {
            if (_timer == null)
            {
                _timer = new System.Threading.Timer(DaemonAction, null, new TimeSpan(0, 0, 20), new TimeSpan(0, 0, 20));
            }

            return Task.CompletedTask;
        }

        private void DaemonAction(object state)
        {
            // In production, this Daemon tracks various things like user alerts.
            // With the incorrect code in the auth attribute (when using AddDbConnectionPool) the database
            // context does not get released from memory correctly.
            using (var scope = _scopeFactory.CreateScope())
            {
                MyDataContext dbCtx = scope.ServiceProvider.GetRequiredService<MyDataContext>();

                // Update something in the database
                dbCtx.Users.AsTracking().First().LastLogin = DateTime.UtcNow;

                dbCtx.SaveChanges();
            }
        }


        public Task StopAsync(CancellationToken cancellationToken)
        {
            _timer?.Change(Timeout.Infinite, Timeout.Infinite);

            return System.Threading.Tasks.Task.CompletedTask;
        }

        public void Dispose()
        {
            _timer?.Dispose();
        }
    }


    [AttributeUsage(AttributeTargets.Method, Inherited = false, AllowMultiple = false)]
    public class AuthAttribute : Microsoft.AspNetCore.Authorization.AuthorizeAttribute, Microsoft.AspNetCore.Mvc.Filters.IAuthorizationFilter
    {

        #region Internal variables

        #endregion


        #region exposed parameters

        #endregion

        public void OnAuthorization(Microsoft.AspNetCore.Mvc.Filters.AuthorizationFilterContext context)
        {
            // In our production scenario, this method does some work every time a user is authenticated,
            // such as extending their session time (there is other logic here).

            // With this using block, if running startup with AddDbContextPool, this code works.
            // with AddDbContext, it causes a database disposed when the page controller tries to access the db
            // (this possibly makes sense if it's accessing and disposing the injected instance.
            // With AddDbContextPool, it causes a memory leak by making the daemon's data context hold references to
            // the ef core lazyloading library (at least that's what the memory dump implies)
            using (MyDataContext dbCtx = context.HttpContext.RequestServices.GetService(typeof(MyDataContext)) as MyDataContext)
            {
                dbCtx.Users.First().LastLogin = DateTime.UtcNow;
                dbCtx.SaveChanges();
            }

            /* The fix (to work with AddDbContext and prevent the leak)
            using (var scope = context.HttpContext.RequestServices.CreateScope())
            {
                var dbCtx = scope.ServiceProvider.GetRequiredService<MyDataContext>();
                dbCtx.Users.First().LastLogin = DateTime.UtcNow;
                dbCtx.SaveChanges();
            }
            */
        }

    }


    [ApiController]
    [Route("Test")]
    public class TestController : ControllerBase
    {
        private readonly MyDataContext _dbCtx;

        public TestController(MyDataContext dbCtx)
        {
            _dbCtx = dbCtx;
        }

        [Auth] // The authattribute defined above
        [HttpPost]
        [ActionName("Test")]
        public IActionResult Test([FromBody] object data)
        {

            _dbCtx.Users.First().LastLogin = DateTime.UtcNow;
            _dbCtx.SaveChanges();

            return Ok(new { });

        }

    }
}

@rexxmagnus
Copy link
Author

I've included a screenshot of Azure's memory statistics after switching our production app to correctly use the database (AddDbContext rather than pool, with no usings on the context) but noticed some strange behaviour. The working memory set (the pale blue line) drops to about 30mb when the app becomes less active (the dip is during the night when no users are on) but the private bytes (the darker blue line above it) barely drops and remains around 400-450mb. Is this indicative of a continuing memory leak or something else?

Screenshot 2021-08-13 15 23 27

@ajcvickers
Copy link
Member

@rexxmagnus This code is manually disposing the context managed by the request scope. The scope should instead be allowed to dispose the context when the scope is disposed.

using (MyDataContext dbCtx = context.HttpContext.RequestServices.GetService(typeof(MyDataContext)) as MyDataContext)
{
    dbCtx.Users.First().LastLogin = DateTime.UtcNow;
    dbCtx.SaveChanges();
}

With regard to holding on to the memory, every EF Core proxy has a reference to the DbContext from which it was queried. This means as long as the proxies are referenced, then the DbContext will also be referenced and not garbage collected. On the other hand, the DbContext should not be holding any references to the proxies once the DbContext has been disposed. If this is happening, then that would be a bug. Is this what you are asserting? If so, we will investigate that.

@rexxmagnus
Copy link
Author

rexxmagnus commented Aug 17, 2021

@ajcvickers Yes, I understand your point about the disposal - once I realised that mistake, I allowed the scope to take control and the memory usage dropped drastically (the chart above shows the working set hovering around 30-50mb overnight now) however the private bytes allocation is still really high and only drops at peculiar times, which made me think the proxies have an issue.

@ajcvickers ajcvickers changed the title EntityFrameworkCore.Internal.LazyLoader using lots of memory Investigate references between proxies and pooled DbContext instances for memory leaks Aug 20, 2021
@ajcvickers ajcvickers added area-dbcontext type-bug and removed closed-no-further-action The issue is closed and no further action is planned. labels Aug 20, 2021
@ajcvickers ajcvickers self-assigned this Aug 20, 2021
@ajcvickers ajcvickers added this to the 6.0.0 milestone Aug 20, 2021
@ajcvickers
Copy link
Member

Notes for triage:

  • Pooled contexts do not retain references to proxies after the context is returned to the pool.
  • Proxies do retain a reference to the context after it is returned to the pool.
    • Attempting to lazy-load using the proxy causes detached loading exception, or no-op if this is disabled.

We should consider disconnecting proxies from the context when the context is returned to the pool, and possibly also when the context is disposed in non-pooling scenarios.

@ajcvickers ajcvickers added the punted-for-7.0 Originally planned for the EF Core 7.0 (EF7) release, but moved out due to resource constraints. label Apr 23, 2022
@kYann
Copy link

kYann commented Aug 14, 2022

Hello !

I'm seeing the same behavior.

@ajcvickers ajcvickers changed the title Investigate references between proxies and pooled DbContext instances for memory leaks Proxies retain a reference to pooled DbContext after it is returned to the pool Dec 24, 2022
@ajcvickers ajcvickers modified the milestones: Backlog, 8.0.0 Dec 24, 2022
@ajcvickers ajcvickers added closed-fixed The issue has been fixed and is/will be included in the release indicated by the issue milestone. and removed consider-for-current-release labels Dec 24, 2022
@ajcvickers ajcvickers modified the milestones: 8.0.0, 8.0.0-preview1 Jan 29, 2023
@ajcvickers ajcvickers modified the milestones: 8.0.0-preview1, 8.0.0 Nov 14, 2023
@ajcvickers ajcvickers removed their assignment Sep 1, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-dbcontext closed-fixed The issue has been fixed and is/will be included in the release indicated by the issue milestone. customer-reported punted-for-6.0 punted-for-7.0 Originally planned for the EF Core 7.0 (EF7) release, but moved out due to resource constraints. type-bug
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants