It's troubling that the leak occurred in the first place. /G
2013/12/15 Lauri Kotilainen <[email protected]> > Which part is troubling? The part about the leak or the part about having > the query plan cache end up constructing the expression? > > If it's the latter, I did try other approaches too. I don't really like > heavy ctors either, but the QPC and NhLinqExpressions are used in a variety > of ways, and all the alternatives I could think of ended up causing > breakage all over the place. This might be due to the fact that I don't > have a very good mental map of the dependencies yet, though. > > I'll file an issue soon-ish with the changes you proposed to the test. > > -Lauri > > On Friday, December 13, 2013 2:41:36 PM UTC+2, Gunnar Liljas wrote: > >> So far it looks OK, although it's a bit troubling that the query plan >> cache does this. I'm not a big fan of heavy constructors, but in this case >> it makes some sense. >> >> I think you can make your test behave correctly (i.e fail when it should) >> by adding a second pass of GC.Collect() >> >> A JIRA issue and an accompanying unit test (for now, remove the >> dependency on SQLite) would be nice. >> >> /G >> >> >> 2013/12/12 Gunnar Liljas <[email protected]> >> >>> Great work, Lauri! >>> >>> I'll do some tests tomorrow, just to give you feedback. >>> >>> /G >>> >>> >>> >>> 2013/12/11 Lauri Kotilainen <[email protected]> >>> >>> Hi, >>>> >>>> I originally posted the description of this issue to the nhusers list: >>>> >>>> https://groups.google.com/d/topic/nhusers/v_6WCod79XE/discussion >>>> >>>> and I won't waste bits by pasting the entire description here unless >>>> it's deemed necessary. Anyway, I think I have a patch that fixes the >>>> session leak, but I don't understand the big picture well enough to >>>> evaluate whether or not it's a safe change. Essentially, what I did was >>>> move most of the code from NhLinqExpression.Translate to the >>>> NhLinqExpression constructor and eliminated the _expression field, making >>>> it a local variable in the ctor. It didn't cause any test failures in the >>>> master branch, and after I backported it to the 3.4.x branch and tested >>>> with the problematic application, the leak is gone (according to ANTS >>>> profiler). >>>> >>>> Here's the patch: >>>> https://gist.github.com/rytmis/3735cfc274e135aae753 >>>> >>>> Unfortunately, even with that patch applied, the unit test in my other >>>> post fails -- even though a heap inspection with WinDBG confirms that the >>>> session is no longer rooted and is eligible for collection. >>>> >>>> What I would like to know at this point is whether the change is likely >>>> to cause performance regressions or other unexpected side effects. >>>> >>>> Thanks, >>>> >>>> -Lauri >>>> >>>> -- >>>> >>>> --- >>>> You received this message because you are subscribed to the Google >>>> Groups "nhibernate-development" group. >>>> To unsubscribe from this group and stop receiving emails from it, send >>>> an email to [email protected]. >>>> For more options, visit https://groups.google.com/groups/opt_out. >>>> >>> >>> >> -- > > --- > You received this message because you are subscribed to the Google Groups > "nhibernate-development" group. > To unsubscribe from this group and stop receiving emails from it, send an > email to [email protected]. > For more options, visit https://groups.google.com/groups/opt_out. > -- --- You received this message because you are subscribed to the Google Groups "nhibernate-development" group. To unsubscribe from this group and stop receiving emails from it, send an email to [email protected]. For more options, visit https://groups.google.com/groups/opt_out.
