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. >> > >
<<Emoticon10.gif>>
