Still attempt to release the session even if in use

Author: fredericDelaporte

src/NHibernate.Test/SystemTransactions/SystemTransactionFixture.cs

@@ -649,7 +649,10 @@ public void CanUseSessionWithManyDependentTransaction(bool explicitFlush)
 		[Test]
 		public void SupportsTransactionTimeout()
 		{
-			Assume.That(TestDialect.SupportsTransactionScopeTimeouts, Is.True);
+			Assume.That(TestDialect.SupportsTransactionScopeTimeouts, Is.True, "The tested dialect is not supported for transaction scope timeouts.");
+			// ODBC always freezes the session during transaction scopes timeouts.
+			Assume.That(Sfi.ConnectionProvider.Driver, Is.Not.InstanceOf(typeof(OdbcDriver)), "ODBC is not supported for transaction scope timeouts.");
+
 			// Test case adapted from https://github.com/kaksmet/NHibBugRepro
 
 			// Create some test data.
@@ -670,7 +673,7 @@ public void SupportsTransactionTimeout()
 				t.Commit();
 			}
 
-			// Setup unhandler exception catcher
+			// Setup unhandled exception catcher.
 			_unhandledExceptions = new ConcurrentBag<object>();
 			AppDomain.CurrentDomain.UnhandledException += CurrentDomain_UnhandledException;
 			try
@@ -697,18 +700,63 @@ public void SupportsTransactionTimeout()
 						// Assume that is a transaction timeout. It may cause various failures, of which some are hard to identify.
 						timeoutsCount++;
 					}
+					// If in need of checking some specific failures, the following code may be used instead:
+					/*
+					catch (Exception ex)
+					{
+						var currentEx = ex;
+						// Depending on where the transaction aborption has broken NHibernate processing, we may
+						// get various exceptions, like directly a TransactionAbortedException with an inner
+						// TimeoutException, or a HibernateException encapsulating a TransactionException with a
+						// timeout, ...
+						bool isTransactionException, isTimeout;
+						do
+						{
+							isTransactionException = currentEx is System.Transactions.TransactionException;
+							isTimeout = isTransactionException && currentEx is TransactionAbortedException;
+							currentEx = currentEx.InnerException;
+						}
+						while (!isTransactionException && currentEx != null);
+						while (!isTimeout && currentEx != null)
+						{
+							isTimeout = currentEx is TimeoutException;
+							currentEx = currentEx?.InnerException;
+						}
+
+						if (!isTimeout)
+						{
+							// We may also get a GenericADOException with an InvalidOperationException stating the
+							// transaction associated to the connection is no more active but not yet suppressed,
+							// and that for executing some SQL, we need to suppress it. That is a weak way of
+							// identifying the case, especially with the many localizations of the message.
+							currentEx = ex;
+							do
+							{
+								isTimeout = currentEx is InvalidOperationException && currentEx.Message.Contains("SQL");
+								currentEx = currentEx?.InnerException;
+							}
+							while (!isTimeout && currentEx != null);
+						}
+
+						if (isTimeout)
+							timeoutsCount++;
+						else
+							throw;
+					}
+					*/
 				}
 
-				// Despite the Thread sleep and the count of entities to load, this test may get the timeout only for slightly
-				// more than 10% of the attempts.
-				Assert.That(timeoutsCount, Is.GreaterThan(5), "The test should have generated more timeouts.");
 				Assert.That(
 					_unhandledExceptions.Count,
 					Is.EqualTo(0),
 					"Unhandled exceptions have occurred: {0}",
 					string.Join(@"
 
 ", _unhandledExceptions));
+
+				// Despite the Thread sleep and the count of entities to load, this test may get the timeout only for slightly
+				// more than 10% of the attempts.
+				Warn.Unless(timeoutsCount, Is.GreaterThan(5), "The test should have generated more timeouts.");
 			}
 			finally
 			{

src/NHibernate.Test/TestDialects/SapSQLAnywhere17TestDialect.cs

@@ -1,4 +1,4 @@
-namespace NHibernate.Test.TestDialects
+namespace NHibernate.Test.TestDialects
 {
 	public class SapSQLAnywhere17TestDialect : TestDialect
 	{
@@ -43,5 +43,11 @@ public SapSQLAnywhere17TestDialect(Dialect.Dialect dialect)
 		/// Does not support SELECT FOR UPDATE 
 		/// </summary>
 		public override bool SupportsSelectForUpdate => false;
+
+		/// <summary>
+		/// SQL Anywhere freezes on transaction scope timeout occuring on concurrent threads, always causing the
+		/// synchronization for end of session processing to timeout.
+		/// </summary>
+		public override bool SupportsTransactionScopeTimeouts => false;
 	}
 }

src/NHibernate/Transaction/AdoNetWithSystemTransactionFactory.cs

@@ -243,9 +243,9 @@ public virtual void Wait()
 					// Remove the block then throw.
 					Unlock();
 					throw new HibernateException(
-						$"Synchronization timeout for transaction completion. Either raise" +
-						$"{Cfg.Environment.SystemTransactionCompletionLockTimeout}, or check all scopes are properly" +
-						$"disposed and/or all direct System.Transaction.Current changes are properly managed.");
+						"A synchronization timeout occurred at transaction completion. Either raise " +
+						$"{Cfg.Environment.SystemTransactionCompletionLockTimeout}, or check all scopes are properly " +
+						"disposed and/or all direct System.Transaction.Current changes are properly managed.");
 				}
 				catch (HibernateException)
 				{
@@ -452,6 +452,7 @@ protected virtual void CompleteTransaction(bool isCommitted)
 				// do an early exit here in such case.
 				if (!IsInActiveTransaction)
 					return;
+				var isSessionProcessing = _session.IsProcessing();
 				try
 				{
 					// Allow transaction completed actions to run while others stay blocked.
@@ -460,22 +461,27 @@ protected virtual void CompleteTransaction(bool isCommitted)
 					// cancelled on a new thread even for non-distributed scopes. So, the session could be doing some processing,
 					// and will not be interrupted until attempting some usage of the connection. See #3355.
 					// Thread safety of a concurrent session BeginProcess is ensured by the Wait performed by BeginProcess.
-					var isProcessing = _session.IsProcessing();
-					if (isProcessing)
+					if (isSessionProcessing)
 					{
 						var timeOutGuard = new Stopwatch();
 						timeOutGuard.Start();
-						while (isProcessing && timeOutGuard.ElapsedMilliseconds < _systemTransactionCompletionLockTimeout)
+						while (isSessionProcessing && timeOutGuard.ElapsedMilliseconds < _systemTransactionCompletionLockTimeout)
 						{
 							// Naïve yield.
 							Thread.Sleep(10);
-							isProcessing = _session.IsProcessing();
+							isSessionProcessing = _session.IsProcessing();
+						}
+						if (isSessionProcessing)
+						{
+							// Throwing would give up attempting to close the session if need be, which may still succeed. So,
+							// just log an error.
+							_logger.Error(
+								"A synchronization timeout occurred at transaction completion: the session is still processing. Attempting " +
+									"to finalize the transaction concurrently, which may cause thread safety failure. You may " +
+									"raise {0} if it is set too low. It may also be a limitation of the data provider, like not " +
+									"supporting transaction scope timeouts occurring on concurrent threads.",
+								Cfg.Environment.SystemTransactionCompletionLockTimeout);
 						}
-						if (isProcessing)
-							throw new HibernateException(
-								$"Synchronization timeout for transaction completion. Either raise" +
-								$"{Cfg.Environment.SystemTransactionCompletionLockTimeout}, or check all scopes are properly" +
-								$"disposed and/or all direct System.Transaction.Current changes are properly managed.");
 					}
 					using (_session.BeginContext())
 					{
@@ -516,6 +522,15 @@ protected virtual void CompleteTransaction(bool isCommitted)
 					// Dispose releases blocked threads by the way.
 					Dispose();
 				}
+
+				if (isSessionProcessing)
+				{
+					throw new HibernateException(
+						"A synchronization timeout occurred at transaction completion: the session was still processing. You may " +
+							$"raise {Cfg.Environment.SystemTransactionCompletionLockTimeout} if it is set too low. It may also " +
+							"be a limitation of the data provider, like not supporting transaction scope timeouts occurring on " +
+							"concurrent threads.");
+				}
 			}
 
 			private static void Cleanup(ISessionImplementor session)