NH-3023 - Cease closing connection from transaction scope completion whenever possible.

Author: fredericDelaporte

src/NHibernate.Test/Async/NHSpecificTest/NH3023/DeadlockConnectionPoolIssueTest.cs

@@ -14,9 +14,11 @@
 using System.IO;
 using System.Linq;
 using System.Text.RegularExpressions;
+using System.Threading;
 using System.Transactions;
 using log4net;
 using log4net.Repository.Hierarchy;
+using NHibernate.Cfg;
 using NHibernate.Dialect;
 using NHibernate.Driver;
 using NHibernate.Engine;
@@ -25,11 +27,19 @@
 namespace NHibernate.Test.NHSpecificTest.NH3023
 {
 	using System.Threading.Tasks;
-	using System.Threading;
 	[TestFixture]
-	public class DeadlockConnectionPoolIssueTestAsync : BugTestCase
+	public class DeadlockConnectionPoolIssueAsync : BugTestCase
 	{
-		private static readonly ILog _log = LogManager.GetLogger(typeof(DeadlockConnectionPoolIssueTestAsync));
+		private static readonly ILog _log = LogManager.GetLogger(typeof(DeadlockConnectionPoolIssueAsync));
+
+		protected virtual bool UseConnectionOnSystemTransactionPrepare => true;
+
+		protected override void Configure(Configuration configuration)
+		{
+			configuration.SetProperty(
+				Cfg.Environment.UseConnectionOnSystemTransactionPrepare,
+				UseConnectionOnSystemTransactionPrepare.ToString());
+		}
 
 		// Uses directly SqlConnection.
 		protected override bool AppliesTo(ISessionFactoryImplementor factory)
@@ -47,27 +57,31 @@ protected override void OnSetUp()
 
 		protected override void OnTearDown()
 		{
+			// Before clearing the pool for dodging pool corruption, we need to wait
+			// for late transaction processing not yet ended.
+			Thread.Sleep(100);
 			//
 			// Hopefully this will clean up the pool so that teardown can succeed
 			//
 			SqlConnection.ClearAllPools();
 
+			RunScript("db-teardown.sql");
+
 			using (var s = OpenSession())
 			{
 				s.CreateQuery("delete from System.Object").ExecuteUpdate();
 			}
-			RunScript("db-teardown.sql");
 		}
 
 		[Theory]
-		public async Task ConnectionPoolCorruptionAfterDeadlockAsync(bool distributed)
+		public async Task ConnectionPoolCorruptionAfterDeadlockAsync(bool distributed, bool disposeSessionBeforeScope)
 		{
 			var tryCount = 0;
 			var id = 1;
-			var missingDeadlock = false;
 			do
 			{
 				tryCount++;
+				var missingDeadlock = false;
 
 				try
 				{
@@ -78,13 +92,13 @@ public async Task ConnectionPoolCorruptionAfterDeadlockAsync(bool distributed)
 					// wrong when disposing a connection from transaction scope completion.
 					// Note that the transaction completion event can execute as soon as the deadlock occurs. It does
 					// not wait for the scope disposal.
-					using (var session = OpenSession())
-					//using (var session = Sfi.WithOptions().ConnectionReleaseMode(ConnectionReleaseMode.OnClose).OpenSession())
-					using (var scope = distributed ? CreateDistributedTransactionScope() : new TransactionScope(TransactionScopeAsyncFlowOption.Enabled))
+					var session = OpenSession();
+					var scope = distributed ? CreateDistributedTransactionScope() : new TransactionScope(TransactionScopeAsyncFlowOption.Enabled);
+					try
 					{
 						_log.Debug("Session and scope opened");
 						session.GetSessionImplementation().Factory.TransactionFactory
-							.EnlistInSystemTransactionIfNeeded(session.GetSessionImplementation());
+							   .EnlistInSystemTransactionIfNeeded(session.GetSessionImplementation());
 						_log.Debug("Session enlisted");
 						try
 						{
@@ -101,17 +115,6 @@ public async Task ConnectionPoolCorruptionAfterDeadlockAsync(bool distributed)
 								// It did what it was supposed to do.
 								//
 								_log.InfoFormat("Expected deadlock on attempt {0}. {1}", tryCount, x.Message);
-
-								// Check who takes time in the disposing
-								var chrono = new Stopwatch();
-								chrono.Start();
-								scope.Dispose();
-								_log.Debug("Scope disposed");
-								Assert.That(chrono.Elapsed, Is.LessThan(TimeSpan.FromSeconds(2)), "Abnormal scope disposal duration");
-								chrono.Restart();
-								session.Dispose();
-								_log.Debug("Session disposed");
-								Assert.That(chrono.Elapsed, Is.LessThan(TimeSpan.FromSeconds(2)), "Abnormal session disposal duration");
 								continue;
 							}
 
@@ -130,7 +133,7 @@ public async Task ConnectionPoolCorruptionAfterDeadlockAsync(bool distributed)
 								new DomainClass
 								{
 									Id = id++,
-									ByteData = new byte[] { 1, 2, 3 }
+									ByteData = new byte[] {1, 2, 3}
 								}));
 
 							await (session.FlushAsync());
@@ -151,6 +154,57 @@ public async Task ConnectionPoolCorruptionAfterDeadlockAsync(bool distributed)
 						scope.Complete();
 						_log.Debug("Scope completed");
 					}
+					finally
+					{
+						// Check who takes time in the disposing
+						var chrono = new Stopwatch();
+						if (disposeSessionBeforeScope)
+						{
+							try
+							{
+								chrono.Start();
+								session.Dispose();
+								_log.Debug("Session disposed");
+								Assert.That(chrono.Elapsed, Is.LessThan(TimeSpan.FromSeconds(2)), "Abnormal session disposal duration");
+							}
+							catch (Exception ex)
+							{
+								// Log in case it gets hidden by the next finally
+								_log.Warn("Session disposal failure", ex);
+								throw;
+							}
+							finally
+							{
+								chrono.Restart();
+								scope.Dispose();
+								_log.Debug("Scope disposed");
+								Assert.That(chrono.Elapsed, Is.LessThan(TimeSpan.FromSeconds(2)), "Abnormal scope disposal duration");
+							}
+						}
+						else
+						{
+							try
+							{
+								chrono.Start();
+								scope.Dispose();
+								_log.Debug("Scope disposed");
+								Assert.That(chrono.Elapsed, Is.LessThan(TimeSpan.FromSeconds(2)), "Abnormal scope disposal duration");
+							}
+							catch (Exception ex)
+							{
+								// Log in case it gets hidden by the next finally
+								_log.Warn("Scope disposal failure", ex);
+								throw;
+							}
+							finally
+							{
+								chrono.Restart();
+								session.Dispose();
+								_log.Debug("Session disposed");
+								Assert.That(chrono.Elapsed, Is.LessThan(TimeSpan.FromSeconds(2)), "Abnormal session disposal duration");
+							}
+						}
+					}
 					_log.Debug("Session and scope disposed");
 				}
 				catch (AssertionException)

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

@@ -641,6 +641,34 @@ public async Task DoNotDeadlockOnAfterTransactionWaitAsync()
 			Assert.That(interceptor.Exception, Is.Null);
 		}
 
+		[Test]
+		public async Task EnforceConnectionUsageRulesOnTransactionCompletionAsync()
+		{
+			var interceptor = new TransactionCompleteUsingConnectionInterceptor();
+			// Do not invert session and scope, it would cause an expected failure when
+			// UseConnectionOnSystemTransactionEvents is false, due to the session being closed.
+			using (var s = Sfi.WithOptions().Interceptor(interceptor).OpenSession())
+			using (var tx = new TransactionScope(TransactionScopeAsyncFlowOption.Enabled))
+			{
+				ForceEscalationToDistributedTx.Escalate();
+				await (s.SaveAsync(new Person()));
+
+				await (s.FlushAsync());
+				tx.Complete();
+			}
+
+			if (UseConnectionOnSystemTransactionPrepare)
+			{
+				Assert.That(interceptor.BeforeException, Is.Null);
+			}
+			else
+			{
+				Assert.That(interceptor.BeforeException, Is.TypeOf<HibernateException>());
+			}
+			// Currently always forbidden, whatever UseConnectionOnSystemTransactionEvents.
+			Assert.That(interceptor.AfterException, Is.TypeOf<HibernateException>());
+		}
+
 		private void DodgeTransactionCompletionDelayIfRequired()
 		{
 			if (Sfi.ConnectionProvider.Driver.HasDelayedDistributedTransactionCompletion)

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

@@ -478,6 +478,35 @@ public async Task DoNotDeadlockOnAfterTransactionWaitAsync()
 			Assert.That(interceptor.Exception, Is.Null);
 		}
 
+		[Test]
+		public async Task EnforceConnectionUsageRulesOnTransactionCompletionAsync()
+		{
+			var interceptor = new TransactionCompleteUsingConnectionInterceptor();
+			// Do not invert session and scope, it would cause an expected failure when
+			// UseConnectionOnSystemTransactionEvents is false, due to the session being closed.
+			using (var s = WithOptions().Interceptor(interceptor).OpenSession())
+			using (var tx = new TransactionScope(TransactionScopeAsyncFlowOption.Enabled))
+			{
+				if (!AutoJoinTransaction)
+					s.JoinTransaction();
+				await (s.SaveAsync(new Person()));
+
+				await (s.FlushAsync());
+				tx.Complete();
+			}
+
+			if (UseConnectionOnSystemTransactionPrepare)
+			{
+				Assert.That(interceptor.BeforeException, Is.Null);
+			}
+			else
+			{
+				Assert.That(interceptor.BeforeException, Is.TypeOf<HibernateException>());
+			}
+			// Currently always forbidden, whatever UseConnectionOnSystemTransactionEvents.
+			Assert.That(interceptor.AfterException, Is.TypeOf<HibernateException>());
+		}
+
 		protected override ISession OpenSession()
 		{
 			if (AutoJoinTransaction)