Handle the case of concurrent session disposal

Author: fredericDelaporte

src/NHibernate/Impl/AbstractSessionImpl.cs

@@ -410,16 +410,31 @@ public bool IsClosed
 		public IDisposable BeginProcess()
 		{
 			return _processHelper.BeginProcess(this);
-		}
-
-		/// <summary>
-		/// If not nested in a call to <c>BeginProcess</c> on this session, set its session id in context.
-		/// </summary>
-		/// <returns>
-		/// If not already processing, an object to dispose for restoring the previous session id.
-		/// Otherwise, <see langword="null" />.
-		/// </returns>
-		public IDisposable BeginContext()
+        }
+
+        /// <summary>
+        /// If not nested in another call to <c>BeginProcess</c> on this session, optionnaly check
+		/// and update the session status, then set its session id in context and flag it as processing.
+        /// </summary>
+		/// <param name="noCheckAndUpdate"><see langword="true" /> to initiate a processing without
+		/// checking and updating the session.</param>
+        /// <returns>
+        /// If not already processing, an object to dispose for signaling the end of the process.
+        /// Otherwise, <see langword="null" />.
+        /// </returns>
+        protected IDisposable BeginProcess(bool noCheckAndUpdate)
+        {
+            return _processHelper.BeginProcess(this, noCheckAndUpdate);
+        }
+
+        /// <summary>
+        /// If not nested in a call to <c>BeginProcess</c> on this session, set its session id in context.
+        /// </summary>
+        /// <returns>
+        /// If not already processing, an object to dispose for restoring the previous session id.
+        /// Otherwise, <see langword="null" />.
+        /// </returns>
+        public IDisposable BeginContext()
 		{
 			return _processHelper.Processing ? null : SessionIdLoggingContext.CreateOrNull(SessionId);
 		}
@@ -442,15 +457,22 @@ public ProcessHelper()
 
 			public bool Processing { get => _processing; }
 
-			public IDisposable BeginProcess(AbstractSessionImpl session)
+			public IDisposable BeginProcess(AbstractSessionImpl session, bool noCheckAndUpdate = false)
 			{
 				if (_processing)
 					return null;
 
 				try
 				{
 					_context = SessionIdLoggingContext.CreateOrNull(session.SessionId);
-					session.CheckAndUpdateSessionStatus();
+					if (noCheckAndUpdate)
+					{
+						session.TransactionContext?.Wait();
+					}
+					else
+					{
+						session.CheckAndUpdateSessionStatus();
+					}
 					_processing = true;
 				}
 				catch

src/NHibernate/Impl/SessionImpl.cs

@@ -1488,13 +1488,12 @@ public void Reconnect(DbConnection conn)
 		/// </summary>
 		public void Dispose()
 		{
-			using (BeginContext())
+			// Ensure we are not disposing concurrently to transaction completion, which would
+			// remove the transaction context.
+			using (BeginProcess(true))
 			{
 				log.Debug("[session-id={0}] running ISession.Dispose()", SessionId);
-				// Ensure we are not disposing concurrently to transaction completion, which would
-				// remove the context. (Do not store it into a local variable before the Wait.)
-				TransactionContext?.Wait();
-				// If the synchronization above is bugged and lets a race condition remaining, we may
+				// If the BeginProcess synchronization is bugged and lets a race condition remaining, we may
 				// blow here with a null ref exception after the null check. We could introduce
 				// a local variable for avoiding it, but that would turn a failure causing an exception
 				// into a failure causing a session and connection leak. So do not do it, better blow away

src/NHibernate/Transaction/AdoNetWithSystemTransactionFactory.cs

@@ -485,7 +485,13 @@ protected virtual void CompleteTransaction(bool isCommitted)
 				if (!IsInActiveTransaction)
 					return;
 				Lock();
-				var isSessionProcessing = _session.IsProcessing();
+				// In case of a rollback due to a timeout, we may have the session disposal running concurrently
+				// to the transaction completion in a way our current locking mechanism cannot fully protect: the
+				// session disposal "BeginProcess" can go through the Wait before it is locked but flag the
+				// session as procesisng after the transaction completion has read it as not processing. To dodge
+				// that case, we consider the session as still processing initially regardless of its actual
+				// status in case of rollback.
+				var isSessionProcessing = !isCommitted || _session.IsProcessing();
 				try
 				{
 					// Allow transaction completed actions to run while others stay blocked.