Commit it please (mark it as Ignore or Explicit).Thanks. 2009/9/10 Ayende Rahien <[email protected]>
> Sorry?I attached the test to my original email > > > On Thu, Sep 10, 2009 at 10:25 PM, Fabio Maulo <[email protected]>wrote: > >> 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 >> > > -- Fabio Maulo
<<Emoticon10.gif>>
