Oren can you commit the test before the fix ?Perhaps we should see some effect and we can find a reasonable solution without disable a feature. The QueryPlanCache is very important especially for those of us are using HQL in mapping instead Criteria.
2009/9/10 Ayende Rahien <[email protected]> > I am not sure about the effect of (a), I will have to check. > I am currently trying to fix the problem using (d), but providing a place > in the session to store those things. > > > On Thu, Sep 10, 2009 at 9:56 PM, Richard Brown (gmail) < > [email protected]> wrote: > >> Option a/ sounds easiest - but how much time does the cache save? >> (Enough to make it worthwhile working on? I'm guessing 'yes'. [image: >> Sad smile emoticon] ) >> >> Option b/ sounds like it would lose all the benefits of the cache unless >> it's used in a tight loop somewhere. >> >> Option c/ sounds workable - is the query/tree easy to clone? (I'm guessing >> it's not trivial) >> >> >> Is there an option d/ for making the ParameterTranslationImpl threadsafe? >> (If I/we could get rid of the call to >> AdjustNamedParameterLocationsForQueryParameters() would it be threadsafe >> again?) >> >> >> *From:* Ayende Rahien <[email protected]> >> *Sent:* Thursday, September 10, 2009 7:27 PM >> *To:* nhibernate-development <[email protected]> >> *Subject:* [nhibernate-development] Re: Thread Safety issue with NH 2.1.x >> >> Okay, correction, I am not sure how to fix this issue. >> ParameterTranslationImpl >> must be stateful for the lifetime of the query. >> The problem is that we must never give the same query to two threads at >> the same time. >> This basically kills our query plan cache purpose. >> >> As I see it, we have three options: >> a/ disable the query cache completely >> b/ make the query cache a per thread query >> c/ clone the query from the cache when we give it out >> >> Thoughts? >> >> On Thu, Sep 10, 2009 at 8:58 PM, Ayende Rahien <[email protected]> wrote: >> >>> I am following on a bug report that I got, and I have confirmed that we >>> have a thread safety issue within NHibernate. The conditions for that >>> are quite esoteric, which is why no one got it so far. I attached a patch >>> the expose the issue. >>> This was introduced in r4679, to fix NH-1908. >>> >>> The problem is that in ParameterTranslationImpl, we get the wrong value. >>> There is a note there that says: >>> >>> // Steve Strong Note: The original Java does not do this decrement; it >>> increments i for >>> // every parameter type. However, within the Loader.GetParameterTypes() >>> method, this introduces >>> // nulls into the paramTypeList array, which in turn causes >>> Loader.ConvertITypesToSqlTypes() to crash >>> // with a null dereference. An alternative fix is to change the Loader >>> to handle the null. I'm >>> // not sure which fix is the most appropriate. >>> // Legacy.FumTest.CompositeIDQuery() shows the bug if you remove the >>> decrement below... >>> >>> That is indeed the error that I am getting most of the time, but that >>> isn't what is bother me. >>> What is bother me is that we have a test that fails only under multi >>> threaded conditions. >>> >>> I tracked it down and it looks like the issue is with the query plan >>> cache. It will serve the same instance of a query plan for several threads >>> at the same time. >>> That is fine on its own, the problem is that there are things there, >>> like ParameterTranslationImpl, which are stateful, and get corrupted when >>> used from multiple threads. >>> I will commit a fix for this particular issue shortly, but it is >>> something that we have to consider as well, because I don't think that we >>> want to take the burden of ensuring that everything works right all the time >>> under multi threading scenarios. >>> >> >> > -- Fabio Maulo
<<Emoticon10.gif>>
