Closed GoogleCodeExporter closed 9 years ago
I just realized that the NHibernate session is not thread save either, so
PLINQO must cache sessions per thread.
Making the private session dictionaries [ThreadStatic] should do the trick.
Here the updated patch.
Original comment by christia...@gmail.com
on 19 Nov 2012 at 1:51
Attachments:
Thanks for these patches. I'll take a look at them and get them committed.
Original comment by bniemyjski
on 19 Nov 2012 at 9:00
Hello,
I finally had some time to look at your patch and I believe it is the correct
change to make after reading this:
http://nhibernate.hibernatingrhinos.com/tags/patterns (search for ThreadStatic)
However, I pulled down the framework samples (in the Framework-Samples branch)
and added a Tracker unit test to try and break this and I couldn't.
Here is the test that I used:
[Test]
public void ThreadSafeGetNamedQueryTest()
{
using (var db = new TrackerDataContext()) {
var t1 = System.Threading.Tasks.Task.Factory.StartNew(() => db.GetUsersWithRoles());
var t2 = System.Threading.Tasks.Task.Factory.StartNew(() => db.GetUsersWithRoles());
var t3 = System.Threading.Tasks.Task.Factory.StartNew(() => db.GetUsersWithRoles());
var users = db.GetUsersWithRoles();
System.Threading.Tasks.Task.WaitAll(t1, t2, t3);
Assert.Greater(users.Count, 0);
}
}
For reference, the method that it's calling does this:
public IList<User> GetUsersWithRoles()
{
IQuery query = Advanced.DefaultSession.GetNamedQuery("GetUsersWithRoles");
return query.List<User>();
}
Is there any chance, you can create a unit test that can break this so I can
verify your changes?
Original comment by bniemyjski
on 27 Nov 2012 at 4:18
I managed to run your test and it *sometimes* failed:
Tracker.Tests.ThreadSafeGetNamedQueryTest:
System.AggregateException : One or more errors occurred.
----> NHibernate.Exceptions.GenericADOException : could not execute query
[ exec [dbo].[GetUsersWithRoles] ]
[SQL: exec [dbo].[GetUsersWithRoles]]
----> System.InvalidOperationException : There is already an open DataReader associated with this Command which must be closed first.
at System.Threading.Tasks.Task.WaitAll(Task[] tasks, Int32 millisecondsTimeout,
CancellationToken cancellationToken)
at System.Threading.Tasks.Task.WaitAll(Task[] tasks, Int32 millisecondsTimeout)
at System.Threading.Tasks.Task.WaitAll(Task[] tasks)
at Tracker.Tests.ThreadSafeGetNamedQueryTest() in
C:\Test\PLINQO-NH-Samples\CSharp\Tracker\Tests.cs:line 243
--GenericADOException
at NHibernate.Loader.Loader.DoList(ISessionImplementor session, QueryParameters
queryParameters)
at NHibernate.Loader.Loader.ListIgnoreQueryCache(ISessionImplementor session,
QueryParameters queryParameters)
at NHibernate.Loader.Loader.List(ISessionImplementor session, QueryParameters
queryParameters, ISet`1 querySpaces, IType[] resultTypes)
at NHibernate.Loader.Custom.CustomLoader.List(ISessionImplementor session,
QueryParameters queryParameters)
at NHibernate.Impl.SessionImpl.ListCustomQuery(ICustomQuery customQuery,
QueryParameters queryParameters, IList results)
at NHibernate.Impl.SessionImpl.List(NativeSQLQuerySpecification spec,
QueryParameters queryParameters, IList results)
at NHibernate.Impl.SessionImpl.List[T](NativeSQLQuerySpecification spec,
QueryParameters queryParameters)
at NHibernate.Impl.SqlQueryImpl.List[T]()
at Tracker.Data.TrackerDataContext.GetUsersWithRoles() in
C:\Test\PLINQO-NH-Samples\CSharp\Tracker\Data\TrackerDataContext.generated.cs:li
ne 214
at Tracker.Tests.<>c__DisplayClassb.<ThreadSafeGetNamedQueryTest>b__7() in
C:\Test\PLINQO-NH-Samples\CSharp\Tracker\Tests.cs:line 240
at System.Threading.Tasks.Task`1.InvokeFuture(Object futureAsObj)
at System.Threading.Tasks.Task.Execute()
--InvalidOperationException
at
System.Data.SqlClient.SqlInternalConnectionTds.ValidateConnectionForExecute(SqlC
ommand command)
at System.Data.SqlClient.SqlCommand.ValidateCommand(String method, Boolean
async)
at System.Data.SqlClient.SqlCommand.RunExecuteReader(CommandBehavior
cmdBehavior, RunBehavior runBehavior, Boolean returnStream, String method,
DbAsyncResult result)
at System.Data.SqlClient.SqlCommand.RunExecuteReader(CommandBehavior
cmdBehavior, RunBehavior runBehavior, Boolean returnStream, String method)
at System.Data.SqlClient.SqlCommand.ExecuteReader(CommandBehavior behavior,
String method)
at System.Data.SqlClient.SqlCommand.ExecuteDbDataReader(CommandBehavior
behavior)
at System.Data.Common.DbCommand.System.Data.IDbCommand.ExecuteReader()
at NHibernate.AdoNet.AbstractBatcher.ExecuteReader(IDbCommand cmd)
at NHibernate.Loader.Loader.GetResultSet(IDbCommand st, Boolean
autoDiscoverTypes, Boolean callable, RowSelection selection,
ISessionImplementor session)
at NHibernate.Loader.Loader.DoQuery(ISessionImplementor session,
QueryParameters queryParameters, Boolean returnProxies)
at
NHibernate.Loader.Loader.DoQueryAndInitializeNonLazyCollections(ISessionImplemen
tor session, QueryParameters queryParameters, Boolean returnProxies)
at NHibernate.Loader.Loader.DoList(ISessionImplementor session, QueryParameters
queryParameters)
However, I believe one should not use one NHibernateDataContext instance in
multiple threads as you do in the test. So it is ok for me that this test fails
I finally managed to build a test that reproduces my issue, however with a
slightly different exception:
Tracker.Tests.ThreadSafeGetNamedQueryMultiContextTest:
System.AggregateException : One or more errors occurred.
----> System.NullReferenceException : Object reference not set to an instance of an object.
at System.Threading.Tasks.Task.WaitAll(Task[] tasks, Int32 millisecondsTimeout,
CancellationToken cancellationToken)
at System.Threading.Tasks.Task.WaitAll(Task[] tasks, Int32 millisecondsTimeout)
at System.Threading.Tasks.Task.WaitAll(Task[] tasks)
at Tracker.Tests.ThreadSafeGetNamedQueryMultiContextTest() in
C:\Test\PLINQO-NH-Samples\CSharp\Tracker\Tests.cs:line 272
--NullReferenceException
at System.Collections.Generic.Dictionary`2.Insert(TKey key, TValue value,
Boolean add)
at
CodeSmith.Data.NHibernate.NHibernateDataContext.NHibernateDataContextAdvanced.ge
t_StateSession()
at
CodeSmith.Data.NHibernate.NHibernateDataContext.NHibernateDataContextAdvanced.ge
t_DefaultSession()
at Tracker.Data.TrackerDataContext.GetUsersWithRoles() in
C:\Test\PLINQO-NH-Samples\CSharp\Tracker\Data\TrackerDataContext.generated.cs:li
ne 212
at
Tracker.Tests.<>c__DisplayClassf.<ThreadSafeGetNamedQueryMultiContextTest>b__e()
in C:\Test\PLINQO-NH-Samples\CSharp\Tracker\Tests.cs:line 261
at System.Threading.Tasks.Task.Execute()
Finally, here the test:
[Test]
public void ThreadSafeGetSessionMultiContextTest()
{
Action action = () =>
{
using (var db = new TrackerDataContext())
{
var session = db.Advanced.DefaultSession; // may cause exception
//var users = db.GetUsersWithRoles(); // this implicitly requires a DefaultSession
//Assert.Greater(users.Count, 0);
}
};
for (int tries = 0; tries < 10000; tries++)
{
var t1 = System.Threading.Tasks.Task.Factory.StartNew(action);
var t2 = System.Threading.Tasks.Task.Factory.StartNew(action);
System.Threading.Tasks.Task.WaitAll(t1, t2);
}
}
After I applied my fix this test passes.
Original comment by christia...@gmail.com
on 27 Nov 2012 at 10:05
Hello,
FYI: I updated the issue on google code with an answer to your post a while
ago without replying to this email. Its state is still WaitingInfo, maybe
you did not see my post. Therefore I write this email.
Original comment by christia...@gmail.com
on 20 Dec 2012 at 10:10
Thanks for this patch. I've updated this issue.
Original comment by bniemyjski
on 20 Dec 2012 at 5:21
Hello,
This has been fixed in the latest nightly build. Thanks for the patch and
reporting this issue.
On a side note, do you know why the ThreadSafeGetNamedQueryTest unit test is
failing?
Original comment by bniemyjski
on 17 Jan 2013 at 4:51
Hello,
The ThreadSafeGetNamedQueryTest fails because multiple threads are using
the same DataContext, and therefore the same NHibernate session.
However, NHibernate sessions are not thread save as stated here:
http://nhibernate.hibernatingrhinos.com/12/nhibernate-and-the-unit-of-work-patte
rn-part-3
This article also explains how to implement a thread save unit of work
pattern.
Original comment by christia...@gmail.com
on 28 Jan 2013 at 9:00
Hello,
Thanks for that information. I'll update the test accordingly.
Original comment by bniemyjski
on 1 Feb 2013 at 6:02
Original issue reported on code.google.com by
christia...@gmail.com
on 19 Nov 2012 at 11:05Attachments: