From f5e59261baa54a0f5691348e9bdc520029db3c0c Mon Sep 17 00:00:00 2001 From: Petr Waclawek Date: Wed, 7 Mar 2018 09:37:44 +0100 Subject: [PATCH 1/3] GH1594: AsyncLocal leak in SystemTransactionContext --- .../NHSpecificTest/GH1594/Entity.cs | 10 +++ .../GH1594/ExecutionContextExtensions.cs | 15 ++++ .../NHSpecificTest/GH1594/Fixture.cs | 68 +++++++++++++++++++ .../NHSpecificTest/GH1594/Mappings.hbm.xml | 10 +++ .../AdoNetWithSystemTransactionFactory.cs | 3 +- 5 files changed, 105 insertions(+), 1 deletion(-) create mode 100644 src/NHibernate.Test/NHSpecificTest/GH1594/Entity.cs create mode 100644 src/NHibernate.Test/NHSpecificTest/GH1594/ExecutionContextExtensions.cs create mode 100644 src/NHibernate.Test/NHSpecificTest/GH1594/Fixture.cs create mode 100644 src/NHibernate.Test/NHSpecificTest/GH1594/Mappings.hbm.xml diff --git a/src/NHibernate.Test/NHSpecificTest/GH1594/Entity.cs b/src/NHibernate.Test/NHSpecificTest/GH1594/Entity.cs new file mode 100644 index 00000000000..676575207f3 --- /dev/null +++ b/src/NHibernate.Test/NHSpecificTest/GH1594/Entity.cs @@ -0,0 +1,10 @@ +using System; + +namespace NHibernate.Test.NHSpecificTest.GH1594 +{ + class Entity + { + public virtual Guid Id { get; set; } + public virtual string Name { get; set; } + } +} diff --git a/src/NHibernate.Test/NHSpecificTest/GH1594/ExecutionContextExtensions.cs b/src/NHibernate.Test/NHSpecificTest/GH1594/ExecutionContextExtensions.cs new file mode 100644 index 00000000000..9f0aa7ed6ce --- /dev/null +++ b/src/NHibernate.Test/NHSpecificTest/GH1594/ExecutionContextExtensions.cs @@ -0,0 +1,15 @@ +using System.Collections; +using System.Threading; + +namespace NHibernate.Test.NHSpecificTest.GH1594 +{ + public static class ExecutionContextExtensions + { + public static int LocalValuesCount(this ExecutionContext c) + { + var f = typeof(ExecutionContext).GetField("_localValues", System.Reflection.BindingFlags.NonPublic | System.Reflection.BindingFlags.Instance); + var d = (IDictionary) f.GetValue(c); + return d?.Count ?? 0; + } + } +} diff --git a/src/NHibernate.Test/NHSpecificTest/GH1594/Fixture.cs b/src/NHibernate.Test/NHSpecificTest/GH1594/Fixture.cs new file mode 100644 index 00000000000..2e0b2ecaa04 --- /dev/null +++ b/src/NHibernate.Test/NHSpecificTest/GH1594/Fixture.cs @@ -0,0 +1,68 @@ +using System.Linq; +using System.Threading; +using System.Transactions; +using NUnit.Framework; + +namespace NHibernate.Test.NHSpecificTest.GH1594 +{ + [TestFixture] + public class Fixture : BugTestCase + { + protected override void OnSetUp() + { + using (var session = OpenSession()) + using (var transaction = session.BeginTransaction()) + { + var e1 = new GH1594.Entity {Name = "Bob"}; + session.Save(e1); + + var e2 = new GH1594.Entity {Name = "Sally"}; + session.Save(e2); + + transaction.Commit(); + } + } + + protected override void OnTearDown() + { + using (var session = OpenSession()) + using (var transaction = session.BeginTransaction()) + { + // The HQL delete does all the job inside the database without loading the entities, but it does + // not handle delete order for avoiding violating constraints if any. Use + // session.Delete("from System.Object"); + // instead if in need of having NHbernate ordering the deletes, but this will cause + // loading the entities in the session. + session.CreateQuery("delete from System.Object").ExecuteUpdate(); + + transaction.Commit(); + } + } + + [Test] + public void ExecutionContextLocalValuesLeak() + { + using (var session = OpenSession()) + { + RunInTransaction(session); + var localValuesCountAfterFirstCall = ExecutionContext.Capture().LocalValuesCount(); + RunInTransaction(session); + var localValuesCountAfterSecondCall = ExecutionContext.Capture().LocalValuesCount(); + Assert.AreEqual(localValuesCountAfterFirstCall, localValuesCountAfterSecondCall); + } + } + + private void RunInTransaction(ISession session) + { + using (var ts = new TransactionScope()) + { + var result = from e in session.Query() + where e.Name == "Bob" + select e; + + result.ToList(); + ts.Complete(); + } + } + } +} diff --git a/src/NHibernate.Test/NHSpecificTest/GH1594/Mappings.hbm.xml b/src/NHibernate.Test/NHSpecificTest/GH1594/Mappings.hbm.xml new file mode 100644 index 00000000000..021e5ab8b62 --- /dev/null +++ b/src/NHibernate.Test/NHSpecificTest/GH1594/Mappings.hbm.xml @@ -0,0 +1,10 @@ + + + + + + + + + diff --git a/src/NHibernate/Transaction/AdoNetWithSystemTransactionFactory.cs b/src/NHibernate/Transaction/AdoNetWithSystemTransactionFactory.cs index bf8f594c281..e973b3cd40c 100644 --- a/src/NHibernate/Transaction/AdoNetWithSystemTransactionFactory.cs +++ b/src/NHibernate/Transaction/AdoNetWithSystemTransactionFactory.cs @@ -187,7 +187,7 @@ public class SystemTransactionContext : ITransactionContext, IEnlistmentNotifica private readonly ManualResetEventSlim _lock = new ManualResetEventSlim(true); private volatile bool _needCompletionLocking = true; // Required for not locking the completion phase itself when locking session usages from concurrent threads. - private readonly AsyncLocal _bypassLock = new AsyncLocal(); + private static readonly AsyncLocal _bypassLock = new AsyncLocal(); private readonly int _systemTransactionCompletionLockTimeout; /// @@ -208,6 +208,7 @@ public SystemTransactionContext( EnlistedTransaction = transaction.Clone(); _systemTransactionCompletionLockTimeout = systemTransactionCompletionLockTimeout; _useConnectionOnSystemTransactionPrepare = useConnectionOnSystemTransactionPrepare; + _bypassLock.Value = false; } /// From 8721f5a1baabf0105c4162cdc6083abf37b6a890 Mon Sep 17 00:00:00 2001 From: Petr Waclawek Date: Wed, 7 Mar 2018 17:11:13 +0100 Subject: [PATCH 2/3] AsyncLocal leak in SystemTransactionContext review fixes --- .../NHSpecificTest/GH1594/ExecutionContextExtensions.cs | 7 ++++++- .../Transaction/AdoNetWithSystemTransactionFactory.cs | 2 +- 2 files changed, 7 insertions(+), 2 deletions(-) diff --git a/src/NHibernate.Test/NHSpecificTest/GH1594/ExecutionContextExtensions.cs b/src/NHibernate.Test/NHSpecificTest/GH1594/ExecutionContextExtensions.cs index 9f0aa7ed6ce..d51c43dac2f 100644 --- a/src/NHibernate.Test/NHSpecificTest/GH1594/ExecutionContextExtensions.cs +++ b/src/NHibernate.Test/NHSpecificTest/GH1594/ExecutionContextExtensions.cs @@ -7,7 +7,12 @@ public static class ExecutionContextExtensions { public static int LocalValuesCount(this ExecutionContext c) { - var f = typeof(ExecutionContext).GetField("_localValues", System.Reflection.BindingFlags.NonPublic | System.Reflection.BindingFlags.Instance); +#if NETSTANDARD1_3 || NETSTANDARD1_6 || NETSATNDARD2_0 || NETCOREAPP2_0 + const string localValuesFieldName = "m_localValues"; +#else + const string localValuesFieldName = "_localValues"; +#endif + var f = typeof(ExecutionContext).GetField(localValuesFieldName, System.Reflection.BindingFlags.NonPublic | System.Reflection.BindingFlags.Instance); var d = (IDictionary) f.GetValue(c); return d?.Count ?? 0; } diff --git a/src/NHibernate/Transaction/AdoNetWithSystemTransactionFactory.cs b/src/NHibernate/Transaction/AdoNetWithSystemTransactionFactory.cs index e973b3cd40c..402ac3db5f3 100644 --- a/src/NHibernate/Transaction/AdoNetWithSystemTransactionFactory.cs +++ b/src/NHibernate/Transaction/AdoNetWithSystemTransactionFactory.cs @@ -208,7 +208,6 @@ public SystemTransactionContext( EnlistedTransaction = transaction.Clone(); _systemTransactionCompletionLockTimeout = systemTransactionCompletionLockTimeout; _useConnectionOnSystemTransactionPrepare = useConnectionOnSystemTransactionPrepare; - _bypassLock.Value = false; } /// @@ -270,6 +269,7 @@ protected virtual void Lock() protected virtual void Unlock() { _lock.Set(); + _bypassLock.Value = false; } /// From 093d598c2e610e33cf38ca0c460d9a41d317a4f2 Mon Sep 17 00:00:00 2001 From: Petr Waclawek Date: Thu, 8 Mar 2018 09:09:39 +0100 Subject: [PATCH 3/3] AsyncLocal leak in SystemTransactionContext test adjustment for .NET Core --- .../NHSpecificTest/GH1594/ExecutionContextExtensions.cs | 6 +++--- src/NHibernate.Test/NHSpecificTest/GH1594/Fixture.cs | 4 ++++ 2 files changed, 7 insertions(+), 3 deletions(-) diff --git a/src/NHibernate.Test/NHSpecificTest/GH1594/ExecutionContextExtensions.cs b/src/NHibernate.Test/NHSpecificTest/GH1594/ExecutionContextExtensions.cs index d51c43dac2f..efc3f52831e 100644 --- a/src/NHibernate.Test/NHSpecificTest/GH1594/ExecutionContextExtensions.cs +++ b/src/NHibernate.Test/NHSpecificTest/GH1594/ExecutionContextExtensions.cs @@ -7,10 +7,10 @@ public static class ExecutionContextExtensions { public static int LocalValuesCount(this ExecutionContext c) { -#if NETSTANDARD1_3 || NETSTANDARD1_6 || NETSATNDARD2_0 || NETCOREAPP2_0 - const string localValuesFieldName = "m_localValues"; -#else +#if NETFX const string localValuesFieldName = "_localValues"; +#else + const string localValuesFieldName = "m_localValues"; #endif var f = typeof(ExecutionContext).GetField(localValuesFieldName, System.Reflection.BindingFlags.NonPublic | System.Reflection.BindingFlags.Instance); var d = (IDictionary) f.GetValue(c); diff --git a/src/NHibernate.Test/NHSpecificTest/GH1594/Fixture.cs b/src/NHibernate.Test/NHSpecificTest/GH1594/Fixture.cs index 2e0b2ecaa04..0f0118cb56a 100644 --- a/src/NHibernate.Test/NHSpecificTest/GH1594/Fixture.cs +++ b/src/NHibernate.Test/NHSpecificTest/GH1594/Fixture.cs @@ -1,6 +1,7 @@ using System.Linq; using System.Threading; using System.Transactions; +using NHibernate.Engine; using NUnit.Framework; namespace NHibernate.Test.NHSpecificTest.GH1594 @@ -8,6 +9,9 @@ namespace NHibernate.Test.NHSpecificTest.GH1594 [TestFixture] public class Fixture : BugTestCase { + protected override bool AppliesTo(ISessionFactoryImplementor factory) => + factory.ConnectionProvider.Driver.SupportsSystemTransactions; + protected override void OnSetUp() { using (var session = OpenSession())