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] <javascript:>> > >> Great work, Lauri! >> >> I'll do some tests tomorrow, just to give you feedback. >> >> /G >> >> >> >> 2013/12/11 Lauri Kotilainen <[email protected] <javascript:>> >> >>> 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]<javascript:> >>> . >>> 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.
