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

Possible Memory Leak in Integration Tests #12440

Closed
jaredcnance opened this Issue Jun 21, 2018 · 4 comments

Comments

Projects
None yet
2 participants
@jaredcnance

jaredcnance commented Jun 21, 2018

After upgrading our Asp.Net Core and EF Core applications from 1.x to 2.1, I am struggling with what appears to be a memory leak. We are running our tests within a container on CircleCI and they started exiting with an unspecified reason. Tests are run serially by xUnit and I can confirm each test and DbContext get disposed after each test.

The active test run was aborted. Reason: 

Attachments:
  /root/project/.../df01d2b1-1dc6-4e7b-b736-7dda6d266062/Sequence_e227a31fef5a47e498eb22f7738e70e1.xml

Total tests: Unknown. Passed: 360. Failed: 0. Skipped: 8.
Test Run Aborted.
Test execution time: 17.0303 Minutes

The active Test Run was aborted because the host process exited unexpectedly while executing following test(s):
My_Test

Running locally on OSX, I will occasionally get crashes like the following:

The active test run was aborted. Reason: ts.framework) FSEventStreamStart: register_with_server: ERROR: f2d_register_rpc() => (null) (-21)
...
Process is terminating due to StackOverflowException.

The test that it fails on is generally different every time, so I started profiling the test app on Windows 10 using Visual Studio Community 2017 15.7.2 and uncovered what appears to be a memory leak. It seems very similar to #10211 except I am not using a LoggerFactory. Seems like it may also be related to #10535, but it's difficult to tell.

It seems that ServiceProviderCache is continuing to grow and may be causing our circle CI container to crash with OOM.

image

Our DbContext is pretty simple with no base method overrides:

public class HostContext : IdentityDbContext<User, Role, Guid>
{
    public HostContext(DbContextOptions<HostContext> options)
        : base(options)
    { }

   // ...
}

And it gets registered like this. Note that it gets registered as a singleton so that the tests can share the same DbContext as the TestServer and they wrap the tests within a transaction that gets rolled back when the test completes.

public IServiceProvider ConfigureServices(IServiceCollection services)
{
    services.AddDbContext<HostContext>(options =>
    {
        options.UseSqlServer(_connectionString).CommandTimeout(600))
        options.EnableSensitiveDataLogging();
    }, ServiceLifetime.Singleton);
}

Most of our tests get constructed and disposed of like this:

public class TestClass : TestFixture<Startup> { /* ... */ }
public class TestFixture<TStartup> : IDisposable where TStartup : class
{
    private readonly TestServer _server;
    private readonly IServiceProvider _services;

    public TestFixture()
    {
        var builder = new WebHostBuilder()
            .UseStartup<TStartup>();

        _server = new TestServer(builder);
        _services = _server.Host.Services;
        Client = _server.CreateClient();
        Context = GetService<HostContext>();
        // ...
    }
    
    protected T GetService<T>() => (T)_services.GetService(typeof(T));
    // ...

    public void Dispose()
    {
        try
        {
            Context.Dispose();
            _server.Dispose();
        }
        catch (Exception) { }
    }
}

I can also confirm that our container gets disposed when the TestServer is disposed:

public void Configure(
    /* ... */ ,
    IApplicationLifetime appLifetime)
{
    // ... 
    appLifetime.ApplicationStopped.Register(() => _container.Dispose());
}

It appears that a new ServiceProvider gets created per test (per DbContext instance) and is referenced by the ServiceProviderCache:

image

The ServiceProviderCache appears to be a true singleton that does not get disposed when the container gets disposed:

image

Is there anyway for me to dump the cache between test runs or prevent the cache from creating new ServiceProviders?

Further technical details

EF Core version: 2.1.0
Database Provider: Microsoft.EntityFrameworkCore.SqlServer
Operating system: Tested on Windows 10, OSX and Docker on Linux
IDE: Visual Studio 2017 15.7.2

@ajcvickers

This comment has been minimized.

Member

ajcvickers commented Jun 21, 2018

@jaredcnance This is what I think is happening. When AddDbContext is called it ensures certain services used by EF are added to the service provider. For example, ILoggerFactory. Since this is done for every context instance, it's equivalent to what is described in #10211.

Registering the context as a singleton is generally not a good idea even in tests since it should never be registered as singleton in any real app. Assuming that you want to keep it this way, probably the best way to tackle this is to take over management of the internal service provider in your tests. For example:

public IServiceProvider ConfigureServices(IServiceCollection services)
{
    var internallServiceProvider = new ServiceCollection()
        .AddEntityFrameworkSqlServer()
       .BuildServiceProvider();

    services.AddDbContext<HostContext>(options =>
    {
        options.UseSqlServer(_connectionString).CommandTimeout(600))
        options.UseInternalServiceProvider(internallServiceProvider);
        options.EnableSensitiveDataLogging();
    }, ServiceLifetime.Singleton);
}

This will stop EF from building and caching the internal service provider for you. You will, of course, get very poor performance because EF will not be caching internal services, but if you want to setup and tear down services for each test then that's the way it will be.

@jaredcnance

This comment has been minimized.

jaredcnance commented Jun 21, 2018

@ajcvickers thanks, that's very helpful. The reason the DbContext is registered as a singleton is so we can wrap our tests within a transaction, like so:

public class TestFixture<TStartup> : IDisposable where TStartup : class
{
    public TestFixture()
    {
        var builder = new WebHostBuilder()
            .UseStartup<TStartup>();

        _server = new TestServer(builder);
        _services = _server.Host.Services;
        Client = _server.CreateClient();
        // -----------------------------------------------
        // this instance needs to be the same as the one
        // resolved by the web app... since they need to share
        // a transaction
        // -----------------------------------------------
        Context = GetService<HostContext>();
        Transaction = Context.Database.BeginTransaction(IsolationLevel.ReadCommitted);
        // ...
    }
    
    protected T GetService<T>() => (T)_services.GetService(typeof(T));
    // ...

    public void Dispose()
    {
        try
        {
            Context.Dispose();
            _server.Dispose();
        }
        catch (Exception) { }
    }
}
public class TestClass : TestFixture {
  public async Task Can_Get_Articles() {
    // arrange
    var article = ArticleFactory.Get();
    DbContext.Articles.Add(article);
    await dbContext.SaveChangesAsync();

    // act
    var result = await _client.GetAsync("/articles");

    // assert ...
  }
}

It would be great if somehow we could access the request scoped instance from our tests, but I'm not sure how to do that except maybe some AsyncLocal wizardry. I suppose it might be possible to use System.Transactions.TransactionScope here since that is now supported and then maybe we don't need to share DbContext instances, but I'll have to try it out. If that works, then I can probably cache my TestServers. Since I'm currently running tests in series, I should be able to cache the TestServers as is, but that would become problematic once we make the jump to parallelize the tests.

Any other suggestions/secrets you might have around sharing a transaction outside of scoped instance? Preferably something that would work even if tests are running in parallel.

UPDATE: So, after hacking together a solution that exposed DbContexts via an AsyncLocal, and then wrapping the test in a System.Transactions.TransactionScope, I discovered it is only supported on Windows sooo that's not going to work.

PlatformNotSupportedException: This platform does not support distributed transactions

The approach I'm taking now is to try to "lift" a custom IServiceScopeFactory out into the test and provide it to the web app so the test and request can share a scope...

@ajcvickers

This comment has been minimized.

Member

ajcvickers commented Jun 22, 2018

@jaredcnance You might want to take a look at the tests in this repo. We use the same pattern of using a transaction that is never committed for tests that mutate the database. However, it doesn't impact the DbContext scope, it only requires using the same DbConnection in the context instance that updates the database and the context instance that reads from the database.

@jaredcnance

This comment has been minimized.

jaredcnance commented Jul 5, 2018

@ajcvickers I did a write up about my issue and solution here: https://nance.io/a-better-story-for-asp-net-core-integration-testing/

I would really appreciate any feedback or suggestions you might have. Also, it would be pretty awesome if something like this was an officially supported feature. Do you think there would be any possibility of this and if so, do you have any suggestions for how I could contribute to making that happen?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment