Clean-up of TypeFactory

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

@@ -780,4 +780,4 @@ protected override void Configure(Configuration configuration)
 		protected override bool AppliesTo(ISessionFactoryImplementor factory)
 			=> base.AppliesTo(factory) && factory.ConnectionProvider.Driver.SupportsEnlistmentWhenAutoEnlistmentIsDisabled;
 	}
-}
+}

src/NHibernate.Test/MultiThreadRunner.cs

@@ -1,81 +1,117 @@
 using System;
+using System.Collections.Concurrent;
+using System.Collections.Generic;
+using System.Linq;
 using System.Threading;
 
 namespace NHibernate.Test
 {
 	public class MultiThreadRunner<T>
 	{
 		public delegate void ExecuteAction(T subject);
-		private readonly int numThreads;
-		private readonly ExecuteAction[] actions;
-		private readonly Random rnd = new Random();
-		private bool running;
-		private int timeout = 1000;
-		private int timeoutBetweenThreadStart = 30;
 
-		public MultiThreadRunner(int numThreads, ExecuteAction[] actions)
+		private readonly int _numThreads;
+		private readonly ExecuteAction[] _actions;
+		private readonly Random _rnd = new Random();
+		private volatile bool _running;
+		private ConcurrentQueue<Exception> _errors = new ConcurrentQueue<Exception>();
+
+		public MultiThreadRunner(int numThreads, params ExecuteAction[] actions)
 		{
-			if(numThreads < 1)
+			if (numThreads < 1)
 			{
-				throw new ArgumentOutOfRangeException("numThreads",numThreads,"Must be GT 1");
+				throw new ArgumentOutOfRangeException(nameof(numThreads), numThreads, "Must be GTE 1");
 			}
 			if (actions == null || actions.Length == 0)
 			{
-				throw new ArgumentNullException("actions");
+				throw new ArgumentNullException(nameof(actions));
 			}
-			foreach (ExecuteAction action in actions)
+			if (actions.Any(action => action == null))
 			{
-				if(action==null)
-					throw new ArgumentNullException("actions", "null delegate");
+				throw new ArgumentNullException(nameof(actions), "null delegate");
 			}
-			this.numThreads = numThreads;
-			this.actions = actions;
+			_numThreads = numThreads;
+			_actions = actions;
 		}
 
-		public int EndTimeout
-		{
-			get { return timeout; }
-			set { timeout = value; }
-		}
+		public int EndTimeout { get; set; } = 1000;
 
-		public int TimeoutBetweenThreadStart
-		{
-			get { return timeoutBetweenThreadStart; }
-			set { timeoutBetweenThreadStart = value; }
-		}
+		public int TimeoutBetweenThreadStart { get; set; } = 30;
 
-		public void Run(T subjectInstance)
-		{
-			running = true;
-			Thread[] t = new Thread[numThreads];
-			for (int i = 0; i < numThreads; i++)
-			{
-				t[i] = new Thread(ThreadProc);
-				t[i].Name = i.ToString();
-				t[i].Start(subjectInstance);
-				if (i > 2)
-					Thread.Sleep(timeoutBetweenThreadStart);
-			}
+		public Exception[] GetErrors() => _errors.ToArray();
+		public void ClearErrors() => _errors = new ConcurrentQueue<Exception>();
 
-			Thread.Sleep(timeout);
+		public int Run(T subjectInstance)
+		{
+			var allThreads = new List<ThreadHolder<T>>();
 
-			// Tell the threads to shut down, then wait until they all
-			// finish.
-			running = false;
-			for (int i = 0; i < numThreads; i++)
+			var launcher = new Thread(
+				() =>
+				{
+					try
+					{
+						for (var i = 0; i < _numThreads; i++)
+						{
+							var threadHolder = new ThreadHolder<T>
+							{
+								Thread = new Thread(ThreadProc) { Name = i.ToString() },
+								Subject = subjectInstance
+							};
+							threadHolder.Thread.Start(threadHolder);
+							allThreads.Add(threadHolder);
+							if (i > 2 && TimeoutBetweenThreadStart > 0)
+								Thread.Sleep(TimeoutBetweenThreadStart);
+						}
+					}
+					catch (Exception e)
+					{
+						_errors.Enqueue(e);
+						throw;
+					}
+				});
+			var totalLoops = 0;
+			_running = true;
+			// Use a separated thread for launching in case too many threads are asked: the inner Start will freeze
+			// but would be able to resume once _running would have been set to false, causing first threads to stop.
+			launcher.Start();
+			// Sleep for the required timeout, taking into account the start delay (if all threads are launchable without
+			// having to wait due to thread starvation).
+			Thread.Sleep(TimeoutBetweenThreadStart * _numThreads + EndTimeout);
+			// Tell the threads to shut down, then wait until they all finish.
+			_running = false;
+			launcher.Join();
+			foreach (var threadHolder in allThreads.Where(t => t != null))
 			{
-				t[i].Join();
+				threadHolder.Thread.Join();
+				totalLoops += threadHolder.LoopsDone;
 			}
+			return totalLoops;
 		}
 
 		private void ThreadProc(object arg)
 		{
-			T subjectInstance = (T) arg;
-			while (running)
+			try
+			{
+				var holder = (ThreadHolder<T>) arg;
+				while (_running)
+				{
+					var actionIdx = _rnd.Next(0, _actions.Length);
+					_actions[actionIdx](holder.Subject);
+					holder.LoopsDone++;
+				}
+			}
+			catch (Exception e)
 			{
-				int actionIdx = rnd.Next(0, actions.Length);
-				actions[actionIdx](subjectInstance);
+				_errors.Enqueue(e);
+				throw;
 			}
 		}
+
+		private class ThreadHolder<TH>
+		{
+			public Thread Thread { get; set; }
+			public int LoopsDone { get; set; }
+			public TH Subject { get; set; }
+		}
 	}
 }

src/NHibernate.Test/SystemTransactions/DistributedSystemTransactionFixture.cs

@@ -345,44 +345,27 @@ public void TransactionInsertLoadWithRollBackTask(bool explicitFlush)
 			}
 		}
 
-		private int _totalCall;
-
-		[Test, Explicit("Test added for NH-1709 (trying to recreate the issue... without luck). If one thread break the test, you can see the result in the console.")]
+		[Test, Explicit("Test added for NH-1709 (trying to recreate the issue... without luck).")]
 		public void MultiThreadedTransaction()
 		{
 			// Test added for NH-1709 (trying to recreate the issue... without luck)
-			// If one thread break the test, you can see the result in the console.
-			((Logger)_log.Logger).Level = log4net.Core.Level.Debug;
-			var actions = new MultiThreadRunner<object>.ExecuteAction[]
-			{
-				delegate
-					{
-						CanRollbackTransaction(false);
-						_totalCall++;
-					},
-				delegate
-					{
-						RollbackOutsideNh(false);
-						_totalCall++;
-					},
-				delegate
-					{
-						TransactionInsertWithRollBackTask(false);
-						_totalCall++;
-					},
-				delegate
-					{
-						TransactionInsertLoadWithRollBackTask(false);
-						_totalCall++;
-					},
-			};
-			var mtr = new MultiThreadRunner<object>(20, actions)
+			var mtr = new MultiThreadRunner<object>(
+				20,
+				o => CanRollbackTransaction(false),
+				o => RollbackOutsideNh(false),
+				o => TransactionInsertWithRollBackTask(false),
+				o => TransactionInsertLoadWithRollBackTask(false))
 			{
 				EndTimeout = 5000,
 				TimeoutBetweenThreadStart = 5
 			};
-			mtr.Run(null);
-			_log.DebugFormat("{0} calls", _totalCall);
+			var totalCalls = mtr.Run(null);
+			_log.DebugFormat("{0} calls", totalCalls);
+			var errors = mtr.GetErrors();
+			if (errors.Length > 0)
+			{
+				Assert.Fail("One or more thread failed, found {0} errors. First exception: {1}", errors.Length, errors[0]);
+			}
 		}
 
 		[Theory]
@@ -820,4 +803,4 @@ public void SessionIsNotEnlisted()
 			}
 		}
 	}
-}
+}

src/NHibernate.Test/TypesTest/TypeFactoryFixture.cs

@@ -73,42 +73,28 @@ public long? GenericInt64
 		}
 
 		private readonly Random rnd = new Random();
-		private int totalCall;
 
-		[Test, Explicit]
+		[Test]
 		public void MultiThreadAccess()
 		{
 			// Test added for NH-1251
-			// If one thread break the test you can see the result in the console.
-			((Logger) log.Logger).Level = log4net.Core.Level.Debug;
-			MultiThreadRunner<object>.ExecuteAction[] actions = new MultiThreadRunner<object>.ExecuteAction[]
+			var mtr = new MultiThreadRunner<object>(
+				100,
+				o => TypeFactory.GetStringType(rnd.Next(1, 50)),
+				o => TypeFactory.GetBinaryType(rnd.Next(1, 50)),
+				o => TypeFactory.GetSerializableType(rnd.Next(1, 50)),
+				o => TypeFactory.GetTypeType(rnd.Next(1, 20)))
 			{
-				delegate(object o)
-					{
-						TypeFactory.GetStringType(rnd.Next(1, 50));
-						totalCall++;
-					},
-				delegate(object o)
-					{
-						TypeFactory.GetBinaryType(rnd.Next(1, 50));
-						totalCall++;
-					},
-				delegate(object o)
-					{
-						TypeFactory.GetSerializableType(rnd.Next(1, 50));
-						totalCall++;
-					},
-				delegate(object o)
-					{
-						TypeFactory.GetTypeType(rnd.Next(1, 20));
-						totalCall++;
-					},
+				EndTimeout = 2000,
+				TimeoutBetweenThreadStart = 2
 			};
-			MultiThreadRunner<object> mtr = new MultiThreadRunner<object>(100, actions);
-			mtr.EndTimeout = 2000;
-			mtr.TimeoutBetweenThreadStart = 2;
-			mtr.Run(null);
-			log.DebugFormat("{0} calls", totalCall);
+			var totalCalls = mtr.Run(null);
+			log.DebugFormat("{0} calls", totalCalls);
+			var errors = mtr.GetErrors();
+			if (errors.Length > 0)
+			{
+				Assert.Fail("One or more thread failed, found {0} errors. First exception: {1}", errors.Length, errors[0]);
+			}
 		}
 
 		[Test]