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

Query: Entities not being released due to leak in query caching #6737

Closed
dkrooke opened this issue Oct 9, 2016 · 24 comments
Closed

Query: Entities not being released due to leak in query caching #6737

dkrooke opened this issue Oct 9, 2016 · 24 comments
Assignees
Labels
closed-fixed The issue has been fixed and is/will be included in the release indicated by the issue milestone. type-bug
Milestone

Comments

@dkrooke
Copy link

dkrooke commented Oct 9, 2016

Apologies if this has been mentioned before:

The problem I am seeing, is that entities are still being held in memory, even after the Context has been disposed.

I have tried the following:

private void Method() {
     Test();
}

 private void Test()
 {
            using (var Context = new ExampleEntities())
            {
                Context.ExampleTable.ToList();  
          }
}

When you view the memory heap after the method Test has completed, ExampleTable rows are still held in memory.

@smitpatel smitpatel changed the title Entities not being rele Entities not being released Oct 10, 2016
@rowanmiller
Copy link
Contributor

Are you running in release mode? And are you sure that garbage collection has run?

@dkrooke
Copy link
Author

dkrooke commented Oct 10, 2016

I was debugging and I'm 90% certain as other objects (non ef) are released when I force a collection.

Sent from my iPhone

On 10 Oct 2016, at 19:46, Rowan Miller <[email protected]mailto:[email protected]> wrote:

Are you running in release mode? And are you sure that garbage collection has run?

You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHubhttps://github.com//issues/6737#issuecomment-252706890, or mute the threadhttps://github.com/notifications/unsubscribe-auth/AVrOYIyD9fnI8LBY3Uy9hqrxvXS9SJVDks5qyofvgaJpZM4KSBYw.


All electronic mail to, from, or within the Telford & Wrekin Council may be the
subject of a request under the Freedom of Information Act 2000 and related
legislation, and therefore may be required to be disclosed to third parties.

Telford & Wrekin Council may monitor email traffic data and also the content of email in conjunction with the relevant policies.

@divega
Copy link
Contributor

divega commented Oct 10, 2016

A few data points on this in case it helps:

  • By default, entities returned in query results are tracked by default in the DbContext you used to execute the query. This implies the DbContext instance contains references to the entities. There are ways to opt out, e.g. call the AsNoTracking() method in the query.
  • By design, DbContext won't release referenced entities when Dispose() is invoked (e.g. a the end of the using block). You will rely on garbage collection to collect the context and all its children objects instead.
  • I can't remember from the top of my mind if this actually applies to this specific case, but in the past I have definitively seen Debug compilations alter the standard scoping rules of variables. Assuming that was happening here, the variable Context could still be in the method stack even after the using block is finalized. In my experience, if you are paying very close attention you will see some bizarre behaviors in Debug that won't actually occur when you compile for Release.

A more reliable way to tell if the entity is still in memory is to look after the Test() and you force a garbage collection. If you can still observe any unexpected behavior when doing that, it would be great if you could provide a full repro.

@dkrooke
Copy link
Author

dkrooke commented Oct 11, 2016

Hi,

Thanks for the comments.

• Unfortunately AsNoTracking() does not help me, as I have found that by using it, the entity relationships are not loaded automatically.

Rightly or wrongly I have found that by doing the following:

class TableA {
[key]
public Guid Oid {get;set;}
public string Code {get;set;}
}

class TableB{
[key]
public Guid Oid {get;set;}
public Guid TableA {get;set;}
public TABLEA TABLEA {get;set;}
public string Code {get;set;}
}

Context.TableA.ToList();
Context.TableB.ToList();

Significantly out performs

Context.TableB.Include(row => row.TABLEA). AsNoTracking().ToList();

On my more complex model, I also got out of memory exceptions using the AsNoTracking() technique, where I didn’t get anywhere close using the first method.

• I first noticed this when I ran my app in release mode. As you may gather I am loading a large amount of data for a year, If I want to load another years data in, I reset all my objects back to null and restart the load process, I noticed that the amount of memory being used was increasing rather than staying at a constant (as best as) level. Eventually, by keep loading data from 1 year to the next, Out Of Memory exceptions are thrown..

• I looked at the memory, when the USING had completed, then when Test() had completed and then after a system.gc.collect() completed. Each time, the objects were still in memory.

I have attached an sample project hopefully demonstrating my issue.
EFTest.zip

You will need to add the Framework and adjust the DB Connection.

When the Main Method (EFTest) or MainWindow Constructor (WPFApplication) has got to the end, there are multiples of the same rows from TableA & TableB still in memory.

It's probably my complete misunderstanding of the garbage collection

Hope this makes sense

@rowanmiller
Copy link
Contributor

Before we get someone to dig into this, I just want to double check that this isn't because the entities are databound to a form? If there is no databinding, then we'll get someone to look at it.

@dkrooke
Copy link
Author

dkrooke commented Oct 11, 2016

The data items are not bound

@rowanmiller
Copy link
Contributor

Thanks, we'll take a look at it.

@rowanmiller rowanmiller added this to the 1.1.0 milestone Oct 12, 2016
@rowanmiller rowanmiller modified the milestones: 1.1.0, 1.2.0 Oct 17, 2016
@ajcvickers
Copy link
Contributor

This seems to be a leak in query caching, which is keeping references to the context. There may be other paths, but the one I have been able to trace is this:

  • MemoryCache uses CacheKey instances (SqlServerCompiledQueryCacheKey in this case)
  • CacheKey instances have an Expression, which in this case is an ConstantExpression holding an EntityQueryable instance
  • EntityQueryable instances hold on to the EntityQueryProvider
  • EntityQueryProvider holds on to the QueryCompiler
  • QueryCompiler is a scoped provider service that uses the current DbContext through several paths

Marking the bug for re-triage.

@ajcvickers ajcvickers removed this from the 1.2.0 milestone Oct 18, 2016
@ajcvickers ajcvickers removed their assignment Oct 18, 2016
@anpete anpete self-assigned this Oct 18, 2016
anpete added a commit that referenced this issue Oct 19, 2016
- Fixed two places where compiled queries were inadvertantly holding onto scoped QueryCompiler state:

1) The query expression used in the compiled query cache key via EntityQueryable<> constant expressions.
2) The compiled query delegate, via a capturing closure in QueryCompiler itself.
anpete added a commit that referenced this issue Oct 19, 2016
- Fixed two places where compiled queries were inadvertently holding onto scoped QueryCompiler state:

1) The query expression used in the compiled query cache key via EntityQueryable<> constant expressions.
2) The compiled query delegate, via a capturing closure in QueryCompiler itself.
@rowanmiller rowanmiller added this to the 1.0.2 milestone Oct 19, 2016
@schwietertj
Copy link

I can confirm with @ajcvickers that the issue is still happening. This seems like a decent bug because it is going to bogart resources/DTUs on Azure until this is patched.

@divega
Copy link
Contributor

divega commented Oct 26, 2016

@anpete just confirm, is the bug fix included in 1.1.0-preview1?

@anpete
Copy link
Contributor

anpete commented Oct 26, 2016

No, didn't make it.

@schwietertj
Copy link

@anpete Is there a time frame for the patch release and if there is a work around? Is this an issue with IDisposable or just the implementation with EF?

@anpete
Copy link
Contributor

anpete commented Oct 27, 2016

@schwietertj The problem was with EF. You can see the details in this PR #6819

I think there may be a workaround, which is to Detach all entities from the leaked DbContext. I.e. The context instance that was used to initially execute the query. cc @ajcvickers who should be able to confirm.

@ajcvickers
Copy link
Contributor

@schwietertj @anpete That should work to release the vast majority of the memory.

@schwietertj
Copy link

@ajcvickers @anpete Thank you for pointing me in a good direction. I will try it out at work tomorrow and let you know what I find.

@schwietertj
Copy link

@ajcvickers @anpete I believe it worked when a variable was declared and set to the entity. In the following instance there would not be an entity to detach and thus a connection will still persist.

dynamic result = new ExpandoObject();
using (var ctx = new CompanyDbContext())
            {
                result.employees = await ctx.Employees.Where(x => x.CompanyId == company_id && x.Active).CountAsync();
            }

I am thinking I will have to wait for the patch to be released and monitor production. The only other work around I can think of would be to set up a service that queries azure for any connection that has been quiet for 30 seconds and kill it.

@anpete
Copy link
Contributor

anpete commented Oct 28, 2016

@schwietertj You can enumerate all of the entities via DbContext.ChangeTracker.Entries()

@divega
Copy link
Contributor

divega commented Oct 28, 2016

@schwietertj are you saying that you are seeing database connections leaked? The original issue that was reported here and that was fixed and for which the workaround applies is about a memory leak and not related to database connections.

@schwietertj
Copy link

@divega Very good point. I have experienced both memory leaks and connections that should be disposed that are not being disposed of in the application.

After a request is made in the mvc project, I can look at the activity monitor in ssms and still see an active process. The process should not be active since the db context has been disposed.

I originally thought it was due to DI the db context. I removed the injection and refactored with the "using" statement.

Please correct me if I am wrong but the database connection should be closed after the using block has been executed.

If need be I can open a new issue if it is deemed to be a separate issue than this one.

@divega
Copy link
Contributor

divega commented Oct 29, 2016

@schwietertj It could be #6581 which causes some connections to be left open. It has been fixed for 1.0.2 (not yet released) and in 1.1.0-preview1. My understanding is that the workarounds for it are (a) enable MARS in the connection string and (b) avoid using Include() with collections and use multiple queries to retrieve the same data instead.

If you think that is not what you are hitting then yes please create a new issue.

@schwietertj
Copy link

@divega I just tried enabling MARS and the connection count increased 8 fold per active user. We also have to use includes since lazy loading isn't implemented. Is there an ETA on the 1.0.2 release?

@Eilon
Copy link
Member

Eilon commented Nov 1, 2016

This patch is approved, please ensure it is merged into the correct branch and building as part of the patch train.

@anpete anpete closed this as completed in bf1c222 Nov 2, 2016
anpete added a commit that referenced this issue Nov 2, 2016
- Fixed two places where compiled queries were inadvertently holding onto scoped QueryCompiler state:

1) The query expression used in the compiled query cache key via EntityQueryable<> constant expressions.
2) The compiled query delegate, via a capturing closure in QueryCompiler itself.
@divega divega added the closed-fixed The issue has been fixed and is/will be included in the release indicated by the issue milestone. label Nov 2, 2016
smitpatel pushed a commit that referenced this issue Nov 8, 2016
- Fixed two places where compiled queries were inadvertently holding onto scoped QueryCompiler state:

1) The query expression used in the compiled query cache key via EntityQueryable<> constant expressions.
2) The compiled query delegate, via a capturing closure in QueryCompiler itself.
@divega divega changed the title Entities not being released Query: Entities not being released due to leak in query caching Dec 12, 2016
Tasteful pushed a commit to Tasteful/EntityFramework that referenced this issue Apr 6, 2017
@joshmouch
Copy link

joshmouch commented Dec 13, 2017

@rowanmiller I know this is an old thread, but about halfway through you ask:

Are you running in release mode?

Does running in debug mode cause some sort of known memory leak? I'm trying to track down an EF memory leak and am grasping at straws.

@ajcvickers
Copy link
Contributor

@joshmouch No known issue like that,

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
closed-fixed The issue has been fixed and is/will be included in the release indicated by the issue milestone. type-bug
Projects
None yet
Development

No branches or pull requests

8 participants