Print Page | Close Window

Query cache memory leak

Printed From: IdeaBlade
Category: DevForce
Forum Name: DevForce 2010
Forum Discription: For .NET 4.0
URL: http://www.ideablade.com/forum/forum_posts.asp?TID=3529
Printed Date: 13-May-2026 at 2:59am


Topic: Query cache memory leak
Posted By: stephenmcd1
Subject: Query cache memory leak
Date Posted: 09-Jul-2012 at 5:22pm
I'm running into an intermittent problem with the query cache.  We've noticed some cases in our application where DevForce will cache a query that involves a closure that captures some memory that we wouldn't expect it to.  An example probably explains it better.  Say I have some class such as this:
//This class has a big memory footprint.  If this class doesn't get garbage collected we will be sad
public class BigClass
{
    //We'll use this property in our query
    public string SomeString { get; private set; }

    //Some hypothetical other stuff in this class that is big and makes us want this class to be cleaned up!
    public byte[] _randomBigData  = new byte[999999];

    public BigClass(string someString)
    {
        SomeString = someString;
    }

    public void DoAQuery()
    {
        var em = new MyEntityManager();

        //Do a simple query on our menus.  We filter based on SomeString which turns into a captured
        //   variable in the expression.  So the expression contains a hard-link to this instance
        em.Menus
            .Where(m => m.Comment == SomeString)
            .ExecuteAsync(op =>
                                {
                                    //Do stuff with the menus that have a comment equal to "Test"
                                });
    }
}

and I use it like this:
//Make the instance
var bigClass = new BigClass("Test");

//Execute the query
bigClass.DoAQuery();

//At this point, we are done with big class.  We'd expect it to start being ready for garbage collection

In most cases, that code works fine and the BigClass instances gets cleaned up.  But we've noticed times in our app when that isn't the case.  After some very basic debugging, I noticed that in most cases, the Entity Manager's query cache contains a bunch of queries that have had the closures replaced with constants (so instead of the expression being m => m.Comment == value(BigClass.SomeString) it will be just m => m.Comment == "Test).  I also noticed that they usually have an "expression wrapper" that isn't compiled and is just the same kind of 'constant replaced' expression.  But in the cases where I've seen the memory leak, the expression wrapper actually said it was compiled and then it has a hard link to the BigClass instance.

I tried to reproduce it in a simple test case but didn't have much luck.  I'm not sure what we might be doing in our real application to cause this.  All the queries should be coming from controls so it would be from the UI thread (plus, I assume I'd get an exception if that wasn't the case).  I can't think of anything else.

Any thoughts?  Is there something I might be doing wrong that would cause this?  Or perhaps something I can do to give you more information.  (unfortunately, our app has gone through some changes recently and so the couple of easy test cases I had in our main app aren't breaking anymore [we are doing more caching so we are also doing far fewer queries]).

Thanks



Replies:
Posted By: DenisK
Date Posted: 10-Jul-2012 at 5:56pm
Hi Stephen,

As part of query execution, a query is first transformed into an expression tree where we visit each expression node and compile and "constantize" (replacing closures with constant values) them when possible. 

You are correct in your debugging in that when an expression is "constantize" it should not cause memory leak. On the other hand, when an expression has a hard link to a class instance, it's possible for a memory leak to happen.

The fix for this is actually pretty simple. Create a variable and resolve the hard link. For example,

change 

em.Menus.Where(m => m.Comment == BigClass.SomeString)

into

var x = BigClass.SomeString;
em.Menus.Where(m => m.Comment == x);

Now this is where I can't seem to connect the dots between your code sample and the bug description because your DoAQuery method is actually doing a simple query with a simple .Where clause already referring to an "x", i.e. SomeString.

Perhaps you could elaborate on what "DoAQuery" is actually doing?

I know you said that you can't reproduce the memory leak in a simple test case but are you able to reproduce the expression wrapper having a hard link to a class instance?


Posted By: stephenmcd1
Date Posted: 11-Jul-2012 at 10:24am
There are two queries that are in play in these examples and I want to be clear about which I'm talking about (this is already a confusing problem....I don't want to add confusion! :-)).  The first query involved is the query that I generate and execute.  The second is the query that actually gets saved in the query cache.  (Perhaps there are more queries behind the scenes as DevForce does the constantization but I don't think they come into play so I'll ignore them)

The one that gets saved to the query cache should never have hard-links to a class instance....that is pretty much guaranteeing a memory leak.  Also (a bit of a sidebar), I have a feeling that could compromise the query cache (I don't know all the details with how it's implemented so I don't know if it's actually a problem or not).  For example, if the cached query is closing over some variable and then the variable changes after the query executes (maybe it's some global static variable that anybody can change), then the cache has now been compromised.  I might execute something like em.Things.Where(t => t.Prop == Globals.TheString)....and when I first executed it, Globals.TheString has the value of "foo" but then I later change TheString to "bar".  Now the query cache thinks that it's seen a query such as Things.Where(t => t.Prop == "bar").

<<end sidebar>>

The query that I build and execute is fine to have the closure since I only expect it to be short-lived.

In the examples above, I can see that the expression wrapper of the first query has a hard-link to my class.  As I said, this is totally acceptable.  And that is what you'd expect, right?  When you asked me if I could reproduce this, where you talking about for this first query or if I could reproduce it for the query that made it into the query cache?  I can't reproduce the cached query having a hard link.  If I could, that would be great.  However, it's trivial to reproduce the hard-link for the first query.

Here is the exact output of the EntityQuery._expressionWrapper.Expression when I look at it in the debugger after executing the query:

(note that I've switched my app to execute against the NorthwindIb so it's now "Region" instead of Menu)
{value(IdeaBlade.EntityModel.EntityQueryProxy`1[MyNamespace.Region]).Where(m => (m.RegionDescription == value(MyNamespace.MainPage+BigClass).SomeString))}

You can see that it captured the BigClass instance via the closure.

Note, this expression wrapper is compiled (ExpressionWrapper.IsCompiled == true).  And so _expressionWrapper._compiledFunc is non-null and also has a reference (via a closure) to the BigClass as well.  In our original profiling from our real app, it is this hard-link in the _compiledFunc that we saw for sure when doing our memory profiling.  The memory link went like this: EntityManager.QueryCache[21] --> ServerEntityQuery.UnderlyingEntityQuery --> EntityQuery<Region>._expressionWrapper --> ExpressionWrapper._compiledFunc --> closure --> [the big class]

Also, using a local variable (e.g. "X") for SomeString doesn't exactly solve the problem.  The expression still closes over an outer variable.  The only thing that changes is that the compiler auto-generates a class and then it's an instance of that class that gets a hard-link.  In a simple case that might be fine since you are only leaking a small class.....but in a more realistic scenario, that can be even worse because then you end up having a memory leak for any other variables that may be closed over.  For example, if I change DoAQuery, to the following I'll end up leaking a different BigClass instance:
public void DoAQuery()
{
    var em = new NorthwindIBEntities();

    var otherBigClass = new BigClass("Number 2");

    //Try to avoid closures by explicitly getting the value of SomeString.  But this doesn't 
    //    avoid the closure...it just changes it.
    var x = SomeString;
    em.Regions
        .Where(m => m.RegionDescription == x)
        .ExecuteAsync(op =>
                            {
                                //I'll reference 'otherBigClass' in the callback function which means it is also captured by a 
                                //  closure. Now 'x' and 'otherBigClass' are both closed over so the compiler generates
                                //  one class to hold them.  DevForce has a hard link because it cares about
                                //  'x' but then that means we also leak otherBigClass :-(
                                otherBigClass.DoSomethingElse();                                          
                            });
}

Thanks for the response.


Posted By: DenisK
Date Posted: 12-Jul-2012 at 7:21pm
Hi Stephen,

Thanks again for the always detailed explanation. We're still investigating so we don't have the right questions to ask for now. We do have a quick suggestion for a workaround and that is to clear the QueryCache by calling .Clear on it periodically or when you know you don't have a need for it anymore.

We'll update you once we have more information.


Posted By: DenisK
Date Posted: 14-Jul-2012 at 12:00pm
Attached is my attempt to create a test solution. I've put some questions and comments there. Let's try to use this as a template for our investigation. As you said, this is already a confusing problem and I hope this will help us be on the same page. Thanks Stephen.

uploads/912/SL_T11985.zip - uploads/912/SL_T11985.zip


Posted By: stephenmcd1
Date Posted: 17-Jul-2012 at 11:39am
Thanks so much for the detailed response.

It looks like the solution you provided is doing thing the same way our real application does.  It's very unfortunate that I'm having a hard time reproducing this problem (either in our real app or in sample application).  I know it can be very difficult to fix something that can't be reproduced.

Also, I'm pretty sure that what you laid out in your "Question 1" is the same thing I was looking at.  Of course, since I can't reproduce it right now, I can't really say that with 100% certainty.

However, I do have some saved PDFs of the output from our memory leak program.  I can provide you with them along with explanations as well as the code involved.  Perhaps that can help explain things.

I have three cases where I saved the PDF output.  I'll pick the simplest case because it is the easiest to understand.

I've uploads/613/NavigationButton_Closure_leak_2_with_temp_variable.zip - attached a PDF version of the memory graph but I'll include it here as text as well so I can add comments. 

AlliantEntity._fkEntityManager - This is a static variable.  It is an Entity Manager that we use for the entire lifetime of the applcation.  Since it's static, anything that this entity manager has a reference to will last forever.

Type: RSS.Alliant.Business.DataModel.DataModelEntityManager - This is our custom type that the T4s make to inherit from EntityManager
Property: (this as EntityManager)._queryCache - This is the property on the Entity Manager that holds a reference to the next thing in the list

Type: IdeaBlade.EntityModel.QueryCache
Property: _existingQueryMap

Type: System.Collections.Generic.Dictionary<Object, DateTime>
Property: entries

System.Collections.Generic.Dictionary<TKey, TValue>+Entry<Object, DateTime>[]
[21].key

IdeaBlade.EntityModel.ServerEntityQuery
<UnderlyingEntityQuery>k__BackingField

IdeaBlade.EntityModel.EntityQuery<DataModel.DataView> - DataView is the type that we were querying for
(this as EntityQuery)._expressionWrapper

IdeaBlade.EntityModel.ExpressionWrapper
_compiledFunc - Normally, _compiledFunc is null.  But in this case, it wasn't.

System.Func<EntityModel.EntityManager, Generic.IEnumerable<DataModel.DataView>>
(this as Delegate)._target

System.Runtime.CompilerServices.Closure - Here we have the closure
Constants

System.Object[]
[1]

System.Object[]
[0]

RSS.Alliant.UI.Components.NavigationButton+<>c__DisplayClass4 - This is a class that was auto-generated by the compiler to capture our local variable
<>4__this

RSS.Alliant.UI.Components.NavigationButton - this is the class that represents "BigClass" in the previous hypothetical cases.  The code that this class executed looks like this:
public class NavigationButton
{
    private void GetDataView()
    {
        //We use the global/static entity manager
        var em = AlliantEntity.FKEntityManager;

        //In trying to fix the leak, we tried making a local copy of the property that the expression will
        //   reference.  But as we know, this doesn't solve the problem.
        string localCopyOfDataView = DataView;

        //This is a very roundabout way to make the query but it is how the real app does it (although, in various
        //   other methods)
        ITypedEntityQuery baseQuery = EntityQuery.Create(typeof(DataView), em);
        EntityQuery<DataView> typedBaseQuery = (EntityQuery<DataView>)baseQuery;
        IEntityQuery<DataView> query = typedBaseQuery.Where(dv => dv.DefaultName == localCopyOfDataView);

        //Execute the query
        em.ExecuteQueryAsync(query, op =>
        {
            //Do stuff

            //At this point, the QueryCache now will have a hard-link to this class (most of the time)
        }, null);
    }
}

Hopefully the memory graph makes sense.  It's in a slightly odd format.  I'm hoping this will help you debug the problem.  If not, we might need to put this issue on the back-burner until I'm able to reproduce it reliably.


Posted By: DenisK
Date Posted: 18-Jul-2012 at 12:18pm
Thanks for the more detailed info Stephen.

I'm going to modify my test solution with this new clues. 

One quick question though, is DataView an EntityType or a string? In your memory graph, DataModel.DataView seems to indicate that it's an EntityType, but yet, in your code sample, you're assigning it to a string??


Posted By: stephenmcd1
Date Posted: 18-Jul-2012 at 1:53pm

I'm glad that information was helpful.


Yeah, I probably should have changed some of the names since that use of 'DataView' is confusing.

DataView is the name of the entity we are trying to query.  It is located in the DataModel namespace.  However, in this code sample, the 'NavigationButton' class above has a string property with the name of DataView.  In this case, the property actually holds the DefaultName (string) of the DataView that we will later loookup.



Posted By: DenisK
Date Posted: 19-Jul-2012 at 7:26pm
Just want to give you a quick update.

I'm still unable to repro even after the new information. No matter what I do, the QueryCache always contains the constantized expression.

One of the senior engineers suggested to try this workaround.

<snip>

        StringContainer.String = DataView;

        //This is a very roundabout way to make the query but it is how the real app does it (although, in various
        //   other methods)
        ITypedEntityQuery baseQuery = EntityQuery.Create(typeof(DataView), em);
        EntityQuery<DataView> typedBaseQuery = (EntityQuery<DataView>)baseQuery;
        IEntityQuery<DataView> query = typedBaseQuery.Where(dv => dv.DefaultName == StringContainer.String);

<snip>

public class StringContainer {
    public static String String;
}


See if this changes how the closure looks like when there's a memory leak. By referring to a static class outside of BigClass, we're hoping to not see the closure still holding on to a BigClass instance.



Print Page | Close Window