Merge pull request #2206 from fredericDelaporte/5.1.x-2172

Fix dependent transaction failure in 5.1.x

appveyor.yml

@@ -1,4 +1,4 @@
-version: 5.1.5.{build}
+version: 5.1.6.{build}
 image: Visual Studio 2017
 environment:
   matrix:

build-common/NHibernate.props

@@ -2,7 +2,7 @@
   <PropertyGroup>
     <VersionMajor Condition="'$(VersionMajor)' == ''">5</VersionMajor>
     <VersionMinor Condition="'$(VersionMinor)' == ''">1</VersionMinor>
-    <VersionPatch Condition="'$(VersionPatch)' == ''">5</VersionPatch>
+    <VersionPatch Condition="'$(VersionPatch)' == ''">6</VersionPatch>
     <VersionSuffix Condition="'$(VersionSuffix)' == ''"></VersionSuffix>
 
     <VersionPrefix>$(VersionMajor).$(VersionMinor).$(VersionPatch)</VersionPrefix>

build-common/common.xml

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

releasenotes.txt

@@ -1,3 +1,11 @@
+Build 5.1.6
+=============================
+
+Release notes - NHibernate - Version 5.1.6
+
+** Bug
+    * #2172 Using DependentTransaction fails
+
 Build 5.1.5
 =============================
 

src/AsyncGenerator.yml

@@ -170,6 +170,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

src/NHibernate.Test/Async/SystemTransactions/DistributedSystemTransactionFixture.cs

@@ -13,12 +13,11 @@
 using System.Threading;
 using System.Transactions;
 using log4net;
-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
 {

src/NHibernate.Test/Async/SystemTransactions/SystemTransactionFixture.cs

@@ -16,9 +16,9 @@
 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
 {

src/NHibernate.Test/SystemTransactions/DistributedSystemTransactionFixture.cs

@@ -3,10 +3,8 @@
 using System.Threading;
 using System.Transactions;
 using log4net;
-using log4net.Repository.Hierarchy;
 using NHibernate.Cfg;
 using NHibernate.Engine;
-using NHibernate.Linq;
 using NHibernate.Test.TransactionTest;
 using NUnit.Framework;
 
@@ -711,6 +709,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)

src/NHibernate.Test/SystemTransactions/SystemTransactionFixture.cs

@@ -6,7 +6,6 @@
 using NHibernate.Cfg;
 using NHibernate.Driver;
 using NHibernate.Engine;
-using NHibernate.Linq;
 using NHibernate.Test.TransactionTest;
 using NUnit.Framework;
 
@@ -509,6 +508,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]

src/NHibernate.Test/TestDialect.cs

@@ -80,5 +80,13 @@ public bool SupportsSqlType(SqlType sqlType)
 		/// Supports the modulo operator on decimal types
 		/// </summary>
 		public virtual bool SupportsModuloOnDecimal => true;
+
+		/// <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;
 	}
 }

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;
 	}
 }

src/NHibernate/AdoNet/ConnectionManager.cs

@@ -456,7 +456,13 @@ public DbCommand CreateCommand()
 		public void EnlistIfRequired(System.Transactions.Transaction transaction)
 		{
 			if (transaction == _currentSystemTransaction)
+			{
+				// Short-circuit after having stored the transaction : they may be equal, but not the same reference.
+				// And the previous one may be an already disposed dependent clone, in which case we need to update
+				// our reference.
+				_currentSystemTransaction = transaction;
 				return;
+			}
 
 			_currentSystemTransaction = transaction;
 

src/NHibernate/Async/Transaction/AdoNetWithSystemTransactionFactory.cs

@@ -16,7 +16,6 @@
 using NHibernate.AdoNet;
 using NHibernate.Engine;
 using NHibernate.Engine.Transaction;
-using NHibernate.Impl;
 using NHibernate.Util;
 
 namespace NHibernate.Transaction