Oren... please upload the test... try to avoid to be Rhino ;) 2009/9/10 Fabio Maulo <[email protected]>
> 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 > -- Fabio Maulo
<<Emoticon10.gif>>
