NH-2176 - Locking the rollback case too. Fixing a bad catch in locking. Adjusted naming for ceasing implying "distributed" while just using system transaction.

fredericDelaporte · fredericDelaporte · commit b867d2575768 · 2017-06-17T15:31:49.000+02:00
diff --git a/src/NHibernate.Test/DebugSessionFactory.cs b/src/NHibernate.Test/DebugSessionFactory.cs
@@ -2,7 +2,6 @@
 using System.Collections.Concurrent;
 using System.Collections.Generic;
 using System.Data.Common;
-using System.Threading;
 using log4net;
 using NHibernate.Cache;
 using NHibernate.Cfg;
@@ -59,18 +58,6 @@ public bool CheckSessionsWereClosed()
 				if (!session.IsOpen)
 					continue;
 
-				if (txContext?.ShouldCloseSessionOnDistributedTransactionCompleted ?? false)
-				{
-					// Delayed rollback not having lock from prepare phase? Give it a chance to complete.
-					Thread.Sleep(100);
-					txContext.WaitOne();
-					if (!session.IsOpen)
-					{
-						_log.Warn($"Test case had a delayed close of session {session.SessionId}.");
-						continue;
-					}
-				}
-
 				_log.Error($"Test case didn't close session {session.SessionId}, closing");
 				allClosed = false;
 				(session as ISession)?.Close();
diff --git a/src/NHibernate.Test/NHSpecificTest/DtcFailures/DtcFailuresFixture.cs b/src/NHibernate.Test/NHSpecificTest/DtcFailures/DtcFailuresFixture.cs
@@ -444,9 +444,6 @@ public void CanDeleteItemInDtc([Values(false, true)] bool explicitFlush)
 				Assert.AreEqual(1, s.Query<Person>().Count(), "Entity not found in database.");
 			}
 
-			// Dodging "latency" due to db still haven't actually committed a distributed tx after scope disposal.
-			Thread.Sleep(100);
-
 			using (var tx = new TransactionScope())
 			{
 				using (var s = OpenSession())
diff --git a/src/NHibernate.Test/NHSpecificTest/NH1054/DummyTransactionFactory.cs b/src/NHibernate.Test/NHSpecificTest/NH1054/DummyTransactionFactory.cs
@@ -18,12 +18,12 @@ public ITransaction CreateTransaction(ISessionImplementor session)
 			throw new NotImplementedException();
 		}
 
-		public void EnlistInDistributedTransactionIfNeeded(ISessionImplementor session)
+		public void EnlistInSystemTransactionIfNeeded(ISessionImplementor session)
 		{
 			throw new NotImplementedException();
 		}
 
-		public bool IsInDistributedActiveTransaction(ISessionImplementor session)
+		public bool IsInActiveSystemTransaction(ISessionImplementor session)
 		{
 			return false;
 		}
diff --git a/src/NHibernate.Test/NHSpecificTest/NH1054/NH1054Fixture.cs b/src/NHibernate.Test/NHSpecificTest/NH1054/NH1054Fixture.cs
@@ -26,7 +26,7 @@ public void AdoNetWithDistributedTransactionFactoryIsDefaultTransactionFactory()
 			Configuration configuration = new Configuration();
 			ISessionFactoryImplementor sessionFactory = (ISessionFactoryImplementor)configuration.BuildSessionFactory();
 
-			Assert.That(sessionFactory.Settings.TransactionFactory, Is.InstanceOf<NHibernate.Transaction.AdoNetWithDistributedTransactionFactory>());
+			Assert.That(sessionFactory.Settings.TransactionFactory, Is.InstanceOf<NHibernate.Transaction.AdoNetWithSystemTransactionFactory>());
 		}
 	}
 }
diff --git a/src/NHibernate/AdoNet/ConnectionManager.cs b/src/NHibernate/AdoNet/ConnectionManager.cs
@@ -69,7 +69,7 @@ public bool IsInActiveExplicitTransaction
 			=> _transaction != null && _transaction.IsActive;
 
 		public bool IsInActiveTransaction
-			=> IsInActiveExplicitTransaction || Factory.TransactionFactory.IsInDistributedActiveTransaction(_session);
+			=> IsInActiveExplicitTransaction || Factory.TransactionFactory.IsInActiveSystemTransaction(_session);
 
 		public bool IsConnected
 			=> _connection != null || _ownConnection;
@@ -399,51 +399,49 @@ public void EnlistIfRequired(System.Transactions.Transaction transaction)
 			_connectionAmbientTransaction = transaction;
 		}
 
-		private bool RequireConnectionSwapInDtc
-			=> _ownConnection && Factory.Dialect.SupportsConcurrentWritingConnectionsInSameTransaction;
-
-		public IDisposable FlushingFromDtcTransaction
+		public IDisposable BeginsFlushingFromSystemTransaction(bool isDistributed)
 		{
-			get
+			var needSwapping = _ownConnection && isDistributed &&
+				Factory.Dialect.SupportsConcurrentWritingConnectionsInSameTransaction;
+			if (needSwapping)
 			{
-				if (RequireConnectionSwapInDtc)
-				{
-					if (Batcher.HasOpenResources)
-						throw new InvalidOperationException("Batcher still has opened ressources at time of Flush from DTC.");
-					// Swap out current connection for avoiding using it concurrently to its own 2PC
-					_dtcBackupConnection = _connection;
-					_dtcBackupConnectionAmbientTransaction = _connectionAmbientTransaction;
-					_connection = null;
-					_connectionAmbientTransaction = null;
-				}
-				_flushingFromDtcTransaction = true;
-				return new StopFlushingFromDtcTransaction(this);
+				if (Batcher.HasOpenResources)
+					throw new InvalidOperationException("Batcher still has opened ressources at time of Flush from DTC.");
+				// Swap out current connection for avoiding using it concurrently to its own 2PC
+				_dtcBackupConnection = _connection;
+				_dtcBackupConnectionAmbientTransaction = _connectionAmbientTransaction;
+				_connection = null;
+				_connectionAmbientTransaction = null;
 			}
+			_flushingFromDtcTransaction = true;
+			return new EndFlushingFromSystemTransaction(this, needSwapping);
 		}
 
-		private class StopFlushingFromDtcTransaction : IDisposable
+		private class EndFlushingFromSystemTransaction : IDisposable
 		{
 			private readonly ConnectionManager _manager;
+			private readonly bool _hasSwappedConnection;
 
-			public StopFlushingFromDtcTransaction(ConnectionManager manager)
+			public EndFlushingFromSystemTransaction(ConnectionManager manager, bool hasSwappedConnection)
 			{
 				_manager = manager;
+				_hasSwappedConnection = hasSwappedConnection;
 			}
 
 			public void Dispose()
 			{
 				_manager._flushingFromDtcTransaction = false;
 
-				if (_manager.RequireConnectionSwapInDtc)
-				{
-					// Release the connection potentially acquired for flushing from DTC.
-					_manager.DisconnectOwnConnection();
-					// Swap back current connection
-					_manager._connection = _manager._dtcBackupConnection;
-					_manager._connectionAmbientTransaction = _manager._dtcBackupConnectionAmbientTransaction;
-					_manager._dtcBackupConnection = null;
-					_manager._dtcBackupConnectionAmbientTransaction = null;
-				}
+				if (!_hasSwappedConnection)
+					return;
+
+				// Release the connection potentially acquired for flushing from DTC.
+				_manager.DisconnectOwnConnection();
+				// Swap back current connection
+				_manager._connection = _manager._dtcBackupConnection;
+				_manager._connectionAmbientTransaction = _manager._dtcBackupConnectionAmbientTransaction;
+				_manager._dtcBackupConnection = null;
+				_manager._dtcBackupConnectionAmbientTransaction = null;
 			}
 		}
 	}
diff --git a/src/NHibernate/Cfg/SettingsFactory.cs b/src/NHibernate/Cfg/SettingsFactory.cs
@@ -393,7 +393,7 @@ private static System.Type CreateLinqQueryProviderType(IDictionary<string, strin
 		private static ITransactionFactory CreateTransactionFactory(IDictionary<string, string> properties)
 		{
 			string className = PropertiesHelper.GetString(
-				Environment.TransactionStrategy, properties, typeof(AdoNetWithDistributedTransactionFactory).FullName);
+				Environment.TransactionStrategy, properties, typeof(AdoNetWithSystemTransactionFactory).FullName);
 			log.Info("Transaction factory: " + className);
 
 			try
diff --git a/src/NHibernate/Driver/SqlClientDriver.cs b/src/NHibernate/Driver/SqlClientDriver.cs
@@ -201,5 +201,11 @@ public override bool SupportsMultipleQueries
 		{
 			get { return true; }
 		}
+
+		/// <summary>
+		/// With read committed snapshot or lower, SQL Server may have not actually already committed the transaction
+		/// right after the scope disposal.
+		/// </summary>
+		public override bool HasDelayedDistributedTransactionCompletion => true;
 	}
 }
diff --git a/src/NHibernate/Impl/AbstractSessionImpl.cs b/src/NHibernate/Impl/AbstractSessionImpl.cs
@@ -437,7 +437,7 @@ protected void AfterOperation(bool success)
 
 		protected void EnlistInAmbientTransactionIfNeeded()
 		{
-			_factory.TransactionFactory.EnlistInDistributedTransactionIfNeeded(this);
+			_factory.TransactionFactory.EnlistInSystemTransactionIfNeeded(this);
 		}
 
 		internal IOuterJoinLoadable GetOuterJoinLoadable(string entityName)
diff --git a/src/NHibernate/Impl/SessionImpl.cs b/src/NHibernate/Impl/SessionImpl.cs
@@ -1560,7 +1560,7 @@ public void Dispose()
 				log.Debug(string.Format("[session-id={0}] running ISession.Dispose()", SessionId));
 				if (TransactionContext != null)
 				{
-					TransactionContext.ShouldCloseSessionOnDistributedTransactionCompleted = true;
+					TransactionContext.ShouldCloseSessionOnSystemTransactionCompleted = true;
 					return;
 				}
 				Dispose(true);
diff --git a/src/NHibernate/Impl/StatelessSessionImpl.cs b/src/NHibernate/Impl/StatelessSessionImpl.cs
@@ -859,7 +859,7 @@ public void Dispose()
 				log.Debug("running IStatelessSession.Dispose()");
 				if (TransactionContext != null)
 				{
-					TransactionContext.ShouldCloseSessionOnDistributedTransactionCompleted = true;
+					TransactionContext.ShouldCloseSessionOnSystemTransactionCompleted = true;
 					return;
 				}
 				Dispose(true);
diff --git a/src/NHibernate/NHibernate.csproj b/src/NHibernate/NHibernate.csproj
@@ -151,6 +151,7 @@
     <Compile Include="ISessionBuilder.cs" />
     <Compile Include="Impl\ISessionCreationOptions.cs" />
     <Compile Include="Tool\hbm2ddl\IDatabaseMetadata.cs" />
+    <Compile Include="Transaction\AdoNetWithSystemTransactionFactory.cs" />
     <Compile Include="Util\DelegateHelper.cs" />
     <Compile Include="Dialect\BitwiseFunctionOperation.cs" />
     <Compile Include="Dialect\DB2Dialect.cs" />
@@ -673,7 +674,6 @@
     <Compile Include="StaleObjectStateException.cs" />
     <Compile Include="Tool\hbm2ddl\SchemaExport.cs" />
     <Compile Include="TransactionException.cs" />
-    <Compile Include="Transaction\AdoNetWithDistributedTransactionFactory.cs" />
     <Compile Include="Transaction\AdoTransaction.cs" />
     <Compile Include="Transaction\AfterTransactionCompletes.cs" />
     <Compile Include="Transaction\ITransactionFactory.cs" />
diff --git a/src/NHibernate/Transaction/AdoNetTransactionFactory.cs b/src/NHibernate/Transaction/AdoNetTransactionFactory.cs
@@ -20,12 +20,12 @@ public ITransaction CreateTransaction(ISessionImplementor session)
 			return new AdoTransaction(session);
 		}
 
-		public void EnlistInDistributedTransactionIfNeeded(ISessionImplementor session)
+		public void EnlistInSystemTransactionIfNeeded(ISessionImplementor session)
 		{
 			// nothing need to do here, we only support local transactions with this factory
 		}
 
-		public bool IsInDistributedActiveTransaction(ISessionImplementor session)
+		public bool IsInActiveSystemTransaction(ISessionImplementor session)
 		{
 			return false;
 		}
diff --git a/src/NHibernate/Transaction/AdoNetWithSystemTransactionFactory.cs b/src/NHibernate/Transaction/AdoNetWithSystemTransactionFactory.cs
diff --git a/src/NHibernate/Transaction/ITransactionContext.cs b/src/NHibernate/Transaction/ITransactionContext.cs
diff --git a/src/NHibernate/Transaction/ITransactionFactory.cs b/src/NHibernate/Transaction/ITransactionFactory.cs

Original file line number	Diff line number	Diff line change
`@@ -444,9 +444,6 @@ public void CanDeleteItemInDtc([Values(false, true)] bool explicitFlush)`
`444`	`444`	`Assert.AreEqual(1, s.Query<Person>().Count(), "Entity not found in database.");`
`445`	`445`	`}`
`446`	`446`
`447`		`- // Dodging "latency" due to db still haven't actually committed a distributed tx after scope disposal.`
`448`		`- Thread.Sleep(100);`
`449`		`-`
`450`	`447`	`using (var tx = new TransactionScope())`
`451`	`448`	`{`
`452`	`449`	`using (var s = OpenSession())`
Original file line number	Diff line number	Diff line change
`@@ -18,12 +18,12 @@ public ITransaction CreateTransaction(ISessionImplementor session)`
`18`	`18`	`throw new NotImplementedException();`
`19`	`19`	`}`
`20`	`20`
`21`		`- public void EnlistInDistributedTransactionIfNeeded(ISessionImplementor session)`
	`21`	`+ public void EnlistInSystemTransactionIfNeeded(ISessionImplementor session)`
`22`	`22`	`{`
`23`	`23`	`throw new NotImplementedException();`
`24`	`24`	`}`
`25`	`25`
`26`		`- public bool IsInDistributedActiveTransaction(ISessionImplementor session)`
	`26`	`+ public bool IsInActiveSystemTransaction(ISessionImplementor session)`
`27`	`27`	`{`
`28`	`28`	`return false;`
`29`	`29`	`}`
Original file line number	Diff line number	Diff line change
`@@ -26,7 +26,7 @@ public void AdoNetWithDistributedTransactionFactoryIsDefaultTransactionFactory()`
`26`	`26`	`Configuration configuration = new Configuration();`
`27`	`27`	`ISessionFactoryImplementor sessionFactory = (ISessionFactoryImplementor)configuration.BuildSessionFactory();`
`28`	`28`
`29`		`- Assert.That(sessionFactory.Settings.TransactionFactory, Is.InstanceOf<NHibernate.Transaction.AdoNetWithDistributedTransactionFactory>());`
	`29`	`+ Assert.That(sessionFactory.Settings.TransactionFactory, Is.InstanceOf<NHibernate.Transaction.AdoNetWithSystemTransactionFactory>());`
`30`	`30`	`}`
`31`	`31`	`}`
`32`	`32`	`}`
Original file line number	Diff line number	Diff line change
`@@ -393,7 +393,7 @@ private static System.Type CreateLinqQueryProviderType(IDictionary<string, strin`
`393`	`393`	`private static ITransactionFactory CreateTransactionFactory(IDictionary<string, string> properties)`
`394`	`394`	`{`
`395`	`395`	`string className = PropertiesHelper.GetString(`
`396`		`- Environment.TransactionStrategy, properties, typeof(AdoNetWithDistributedTransactionFactory).FullName);`
	`396`	`+ Environment.TransactionStrategy, properties, typeof(AdoNetWithSystemTransactionFactory).FullName);`
`397`	`397`	`log.Info("Transaction factory: " + className);`
`398`	`398`
`399`	`399`	`try`
Original file line number	Diff line number	Diff line change
`@@ -201,5 +201,11 @@ public override bool SupportsMultipleQueries`
`201`	`201`	`{`
`202`	`202`	`get { return true; }`
`203`	`203`	`}`
	`204`	`+`
	`205`	`+ /// <summary>`
	`206`	`+ /// With read committed snapshot or lower, SQL Server may have not actually already committed the transaction`
	`207`	`+ /// right after the scope disposal.`
	`208`	`+ /// </summary>`
	`209`	`+ public override bool HasDelayedDistributedTransactionCompletion => true;`
`204`	`210`	`}`
`205`	`211`	`}`
Original file line number	Diff line number	Diff line change
`@@ -437,7 +437,7 @@ protected void AfterOperation(bool success)`
`437`	`437`
`438`	`438`	`protected void EnlistInAmbientTransactionIfNeeded()`
`439`	`439`	`{`
`440`		`- _factory.TransactionFactory.EnlistInDistributedTransactionIfNeeded(this);`
	`440`	`+ _factory.TransactionFactory.EnlistInSystemTransactionIfNeeded(this);`
`441`	`441`	`}`
`442`	`442`
`443`	`443`	`internal IOuterJoinLoadable GetOuterJoinLoadable(string entityName)`
Original file line number	Diff line number	Diff line change
`@@ -1560,7 +1560,7 @@ public void Dispose()`
`1560`	`1560`	`log.Debug(string.Format("[session-id={0}] running ISession.Dispose()", SessionId));`
`1561`	`1561`	`if (TransactionContext != null)`
`1562`	`1562`	`{`
`1563`		`- TransactionContext.ShouldCloseSessionOnDistributedTransactionCompleted = true;`
	`1563`	`+ TransactionContext.ShouldCloseSessionOnSystemTransactionCompleted = true;`
`1564`	`1564`	`return;`
`1565`	`1565`	`}`
`1566`	`1566`	`Dispose(true);`
Original file line number	Diff line number	Diff line change
`@@ -859,7 +859,7 @@ public void Dispose()`
`859`	`859`	`log.Debug("running IStatelessSession.Dispose()");`
`860`	`860`	`if (TransactionContext != null)`
`861`	`861`	`{`
`862`		`- TransactionContext.ShouldCloseSessionOnDistributedTransactionCompleted = true;`
	`862`	`+ TransactionContext.ShouldCloseSessionOnSystemTransactionCompleted = true;`
`863`	`863`	`return;`
`864`	`864`	`}`
`865`	`865`	`Dispose(true);`
Original file line number	Diff line number	Diff line change
`@@ -20,12 +20,12 @@ public ITransaction CreateTransaction(ISessionImplementor session)`
`20`	`20`	`return new AdoTransaction(session);`
`21`	`21`	`}`
`22`	`22`
`23`		`- public void EnlistInDistributedTransactionIfNeeded(ISessionImplementor session)`
	`23`	`+ public void EnlistInSystemTransactionIfNeeded(ISessionImplementor session)`
`24`	`24`	`{`
`25`	`25`	`// nothing need to do here, we only support local transactions with this factory`
`26`	`26`	`}`
`27`	`27`
`28`		`- public bool IsInDistributedActiveTransaction(ISessionImplementor session)`
	`28`	`+ public bool IsInActiveSystemTransaction(ISessionImplementor session)`
`29`	`29`	`{`
`30`	`30`	`return false;`
`31`	`31`	`}`