Merge pull request #2207 from fredericDelaporte/5.0.x-2172

Fix dependent transaction failure in 5.0.x
nhibernate · Sep 14, 2019 · 0c4524e · 0c4524e
2 parents 6e6e402 + e0d6eac
commit 0c4524e
Show file tree

Hide file tree

Showing 13 changed files with 317 additions and 20 deletions.
diff --git a/build-common/NHibernate.props b/build-common/NHibernate.props
@@ -2,7 +2,7 @@
   <PropertyGroup>
     <VersionMajor Condition="'$(VersionMajor)' == ''">5</VersionMajor>
     <VersionMinor Condition="'$(VersionMinor)' == ''">0</VersionMinor>
-    <VersionPatch Condition="'$(VersionPatch)' == ''">7</VersionPatch>
+    <VersionPatch Condition="'$(VersionPatch)' == ''">8</VersionPatch>
     <VersionSuffix Condition="'$(VersionSuffix)' == ''"></VersionSuffix>
 
     <VersionPrefix>$(VersionMajor).$(VersionMinor).$(VersionPatch)</VersionPrefix>

diff --git a/build-common/common.xml b/build-common/common.xml
@@ -30,8 +30,8 @@
 
 	<!-- This is used only for build folder -->
 	<!-- TODO: Either remove or refactor to use NHibernate.props -->
-	<property name="project.version" value="5.0.7" overwrite="false" />
-	<property name="project.version.numeric" value="5.0.7" overwrite="false" />
+	<property name="project.version" value="5.0.8" overwrite="false" />
+	<property name="project.version.numeric" value="5.0.8" overwrite="false" />
 
 	<!-- properties used to connect to database for testing -->
 	<include buildfile="nhibernate-properties.xml" />

diff --git a/releasenotes.txt b/releasenotes.txt
@@ -1,4 +1,12 @@
-Build 5.0.7
+Build 5.0.8
+=============================
+
+Release notes - NHibernate - Version 5.0.8
+
+** Bug
+    * #2172 Using DependentTransaction fails
+
+Build 5.0.7
 =============================
 
 Release notes - NHibernate - Version 5.0.7

diff --git a/src/AsyncGenerator.yml b/src/AsyncGenerator.yml
@@ -164,6 +164,18 @@
   applyChanges: true
   analyzation:
     methodConversion:
+    - conversion: Ignore
+      name: CanUseDependentTransaction
+      containingTypeName: DistributedSystemTransactionFixture
+    - conversion: Ignore
+      name: CanUseSessionWithManyDependentTransaction
+      containingTypeName: DistributedSystemTransactionFixture
+    - conversion: Ignore
+      name: CanUseDependentTransaction
+      containingTypeName: SystemTransactionFixture
+    - conversion: Ignore
+      name: CanUseSessionWithManyDependentTransaction
+      containingTypeName: SystemTransactionFixture
     - conversion: Copy
       hasAttributeName: OneTimeSetUpAttribute
     - conversion: Copy

diff --git a/src/NHibernate.Test/Async/SystemTransactions/DistributedSystemTransactionFixture.cs b/src/NHibernate.Test/Async/SystemTransactions/DistributedSystemTransactionFixture.cs
@@ -16,9 +16,9 @@
 using log4net.Repository.Hierarchy;
 using NHibernate.Cfg;
 using NHibernate.Engine;
-using NHibernate.Linq;
 using NHibernate.Test.TransactionTest;
 using NUnit.Framework;
+using NHibernate.Linq;
 
 namespace NHibernate.Test.SystemTransactions
 {
@@ -780,4 +780,4 @@ protected override void Configure(Configuration configuration)
 		protected override bool AppliesTo(ISessionFactoryImplementor factory)
 			=> base.AppliesTo(factory) && factory.ConnectionProvider.Driver.SupportsEnlistmentWhenAutoEnlistmentIsDisabled;
 	}
-}
+}
diff --git a/src/NHibernate.Test/Async/SystemTransactions/SystemTransactionFixture.cs b/src/NHibernate.Test/Async/SystemTransactions/SystemTransactionFixture.cs
@@ -14,10 +14,11 @@
 using System.Threading;
 using System.Transactions;
 using NHibernate.Cfg;
+using NHibernate.Driver;
 using NHibernate.Engine;
-using NHibernate.Linq;
 using NHibernate.Test.TransactionTest;
 using NUnit.Framework;
+using NHibernate.Linq;
 
 namespace NHibernate.Test.SystemTransactions
 {
@@ -529,4 +530,4 @@ public class SystemTransactionWithoutAutoJoinTransactionAsync : SystemTransactio
 	{
 		protected override bool AutoJoinTransaction => false;
 	}
-}
+}
diff --git a/src/NHibernate.Test/SystemTransactions/DistributedSystemTransactionFixture.cs b/src/NHibernate.Test/SystemTransactions/DistributedSystemTransactionFixture.cs
@@ -6,7 +6,6 @@
 using log4net.Repository.Hierarchy;
 using NHibernate.Cfg;
 using NHibernate.Engine;
-using NHibernate.Linq;
 using NHibernate.Test.TransactionTest;
 using NUnit.Framework;
 
@@ -728,6 +727,117 @@ public void AdditionalJoinDoesNotThrow()
 			}
 		}
 
+		[Theory]
+		public void CanUseDependentTransaction(bool explicitFlush)
+		{
+			if (!TestDialect.SupportsDependentTransaction)
+				Assert.Ignore("Dialect does not support dependent transactions");
+			IgnoreIfUnsupported(explicitFlush);
+
+			try
+			{
+				using (var committable = new CommittableTransaction())
+				{
+					System.Transactions.Transaction.Current = committable;
+					using (var clone = committable.DependentClone(DependentCloneOption.RollbackIfNotComplete))
+					{
+						System.Transactions.Transaction.Current = clone;
+
+						using (var s = OpenSession())
+						{
+							if (!AutoJoinTransaction)
+								s.JoinTransaction();
+							s.Save(new Person());
+
+							if (explicitFlush)
+								s.Flush();
+							clone.Complete();
+						}
+					}
+
+					System.Transactions.Transaction.Current = committable;
+					committable.Commit();
+				}
+			}
+			finally
+			{
+				System.Transactions.Transaction.Current = null;
+			}
+		}
+
+		[Theory]
+		public void CanUseSessionWithManyDependentTransaction(bool explicitFlush)
+		{
+			if (!TestDialect.SupportsDependentTransaction)
+				Assert.Ignore("Dialect does not support dependent transactions");
+			IgnoreIfUnsupported(explicitFlush);
+
+			try
+			{
+				using (var s = Sfi.WithOptions().ConnectionReleaseMode(ConnectionReleaseMode.OnClose).OpenSession())
+				{
+					using (var committable = new CommittableTransaction())
+					{
+						System.Transactions.Transaction.Current = committable;
+						using (var clone = committable.DependentClone(DependentCloneOption.RollbackIfNotComplete))
+						{
+							System.Transactions.Transaction.Current = clone;
+							if (!AutoJoinTransaction)
+								s.JoinTransaction();
+							// Acquire the connection
+							var count = s.Query<Person>().Count();
+							Assert.That(count, Is.EqualTo(0), "Unexpected initial entity count.");
+							clone.Complete();
+						}
+
+						using (var clone = committable.DependentClone(DependentCloneOption.RollbackIfNotComplete))
+						{
+							System.Transactions.Transaction.Current = clone;
+							if (!AutoJoinTransaction)
+								s.JoinTransaction();
+							s.Save(new Person());
+
+							if (explicitFlush)
+								s.Flush();
+
+							clone.Complete();
+						}
+
+						using (var clone = committable.DependentClone(DependentCloneOption.RollbackIfNotComplete))
+						{
+							System.Transactions.Transaction.Current = clone;
+							if (!AutoJoinTransaction)
+								s.JoinTransaction();
+							var count = s.Query<Person>().Count();
+							Assert.That(count, Is.EqualTo(1), "Unexpected entity count after committed insert.");
+							clone.Complete();
+						}
+
+						System.Transactions.Transaction.Current = committable;
+						committable.Commit();
+					}
+				}
+			}
+			finally
+			{
+				System.Transactions.Transaction.Current = null;
+			}
+
+			DodgeTransactionCompletionDelayIfRequired();
+
+			using (var s = OpenSession())
+			{
+				using (var tx = new TransactionScope())
+				{
+					if (!AutoJoinTransaction)
+						s.JoinTransaction();
+					var count = s.Query<Person>().Count();
+					Assert.That(count, Is.EqualTo(1), "Unexpected entity count after global commit.");
+					tx.Complete();
+				}
+			}
+		}
+
 		private void DodgeTransactionCompletionDelayIfRequired()
 		{
 			if (Sfi.ConnectionProvider.Driver.HasDelayedDistributedTransactionCompletion)
@@ -820,4 +930,4 @@ public void SessionIsNotEnlisted()
 			}
 		}
 	}
-}
+}
diff --git a/src/NHibernate.Test/SystemTransactions/SystemTransactionFixture.cs b/src/NHibernate.Test/SystemTransactions/SystemTransactionFixture.cs
@@ -4,8 +4,8 @@
 using System.Threading;
 using System.Transactions;
 using NHibernate.Cfg;
+using NHibernate.Driver;
 using NHibernate.Engine;
-using NHibernate.Linq;
 using NHibernate.Test.TransactionTest;
 using NUnit.Framework;
 
@@ -502,6 +502,120 @@ public void AdditionalJoinDoesNotThrow()
 				Assert.DoesNotThrow(() => s.JoinTransaction());
 			}
 		}
+
+		[Theory]
+		public void CanUseDependentTransaction(bool explicitFlush)
+		{
+			if (!TestDialect.SupportsDependentTransaction)
+				Assert.Ignore("Dialect does not support dependent transactions");
+			IgnoreIfUnsupported(explicitFlush);
+
+			try
+			{
+				using (var committable = new CommittableTransaction())
+				{
+					System.Transactions.Transaction.Current = committable;
+					using (var clone = committable.DependentClone(DependentCloneOption.RollbackIfNotComplete))
+					{
+						System.Transactions.Transaction.Current = clone;
+
+						using (var s = OpenSession())
+						{
+							if (!AutoJoinTransaction)
+								s.JoinTransaction();
+							s.Save(new Person());
+
+							if (explicitFlush)
+								s.Flush();
+							clone.Complete();
+						}
+					}
+
+					System.Transactions.Transaction.Current = committable;
+					committable.Commit();
+				}
+			}
+			finally
+			{
+				System.Transactions.Transaction.Current = null;
+			}
+		}
+
+		[Theory]
+		public void CanUseSessionWithManyDependentTransaction(bool explicitFlush)
+		{
+			if (!TestDialect.SupportsDependentTransaction)
+				Assert.Ignore("Dialect does not support dependent transactions");
+			IgnoreIfUnsupported(explicitFlush);
+			// ODBC with SQL-Server always causes system transactions to go distributed, which causes their transaction completion to run
+			// asynchronously. But ODBC enlistment also check the previous transaction in a way that do not guard against it
+			// being concurrently disposed of. See https://github.com/nhibernate/nhibernate-core/pull/1505 for more details.
+			if (Sfi.ConnectionProvider.Driver is OdbcDriver)
+				Assert.Ignore("ODBC sometimes fails on second scope by checking the previous transaction status, which may yield an object disposed exception");
+
+			try
+			{
+				using (var s = WithOptions().ConnectionReleaseMode(ConnectionReleaseMode.OnClose).OpenSession())
+				{
+					using (var committable = new CommittableTransaction())
+					{
+						System.Transactions.Transaction.Current = committable;
+						using (var clone = committable.DependentClone(DependentCloneOption.RollbackIfNotComplete))
+						{
+							System.Transactions.Transaction.Current = clone;
+							if (!AutoJoinTransaction)
+								s.JoinTransaction();
+							// Acquire the connection
+							var count = s.Query<Person>().Count();
+							Assert.That(count, Is.EqualTo(0), "Unexpected initial entity count.");
+							clone.Complete();
+						}
+
+						using (var clone = committable.DependentClone(DependentCloneOption.RollbackIfNotComplete))
+						{
+							System.Transactions.Transaction.Current = clone;
+							if (!AutoJoinTransaction)
+								s.JoinTransaction();
+							s.Save(new Person());
+
+							if (explicitFlush)
+								s.Flush();
+
+							clone.Complete();
+						}
+
+						using (var clone = committable.DependentClone(DependentCloneOption.RollbackIfNotComplete))
+						{
+							System.Transactions.Transaction.Current = clone;
+							if (!AutoJoinTransaction)
+								s.JoinTransaction();
+							var count = s.Query<Person>().Count();
+							Assert.That(count, Is.EqualTo(1), "Unexpected entity count after committed insert.");
+							clone.Complete();
+						}
+
+						System.Transactions.Transaction.Current = committable;
+						committable.Commit();
+					}
+				}
+			}
+			finally
+			{
+				System.Transactions.Transaction.Current = null;
+			}
+
+			using (var s = OpenSession())
+			{
+				using (var tx = new TransactionScope())
+				{
+					if (!AutoJoinTransaction)
+						s.JoinTransaction();
+					var count = s.Query<Person>().Count();
+					Assert.That(count, Is.EqualTo(1), "Unexpected entity count after global commit.");
+					tx.Complete();
+				}
+			}
+		}
 	}
 
 	[TestFixture]
@@ -539,4 +653,4 @@ public void SessionIsNotEnlisted()
 			}
 		}
 	}
-}
+}
diff --git a/src/NHibernate.Test/TestDialect.cs b/src/NHibernate.Test/TestDialect.cs
@@ -72,5 +72,13 @@ public bool SupportsSqlType(SqlType sqlType)
 				return false;
 			}
 		}
+
+		/// <summary>
+		/// Some databases fail with dependent transaction, typically when their driver tries to access the transaction
+		/// state from its two PC: the dependent transaction is meant to be disposed of before completing the actual
+		/// transaction, so it is usually disposed at this point, and its state cannot be read. (Drivers should always
+		/// clone transactions for avoiding this trouble.)
+		/// </summary>
+		public virtual bool SupportsDependentTransaction => true;
 	}
 }
diff --git a/src/NHibernate.Test/TestDialects/PostgreSQL83TestDialect.cs b/src/NHibernate.Test/TestDialects/PostgreSQL83TestDialect.cs
@@ -16,5 +16,11 @@ public override bool SupportsNullCharactersInUtfStrings
         {
             get { return false; }
         }
+
+		/// <summary>
+		/// Npgsql does not clone the transaction in its context, and uses it in its prepare phase. When that was a
+		/// dependent transaction, it is then usually already disposed of, causing Npgsql to crash.
+		/// </summary>
+		public override bool SupportsDependentTransaction => false;
 	}
 }