Monthly Archives: August 2014

Unit Testing Private Methods

Some very smart and experienced people say not to test private methods. Fair enough. The reasons are good – read here.

This post is not going to buy into that debate. It’s purpose is to demonstrate how to test private methods. The answer is painfully obvious – use reflection.

In my StringExtensions class, I have a private static helper method called GetIndexOfNthCharHelper. I wanted to unit test it (hey it’s my API and I can control it, so some of the Just Say No reasons go away). This is the test I wrote:

[TestMethod]
public void GetIndexOfNthCharHelperDoes()
{
	//  Arrange
	var type = typeof (StringExtensions);
	var getIndexOfNthCharHelperMethod = type.GetMethod("GetIndexOfNthCharHelper", BindingFlags.NonPublic | BindingFlags.Static);

	//  Act
	int index = (int)getIndexOfNthCharHelperMethod.Invoke(null, new object[] { path, 2, 0, Path.DirectorySeparatorChar });

	//  Assert                        
	Assert.AreEqual(15, index);
}

Not too complex. But I do heed the warnings of others and definitely wouldn’t be too keen to unit test a private method of a 3rd party API over which I have no control.

Lock – the Good, the Bad and the Enhanced

The Good

The lock keyword in C# is very useful for writing thread-safe code. However, it does have its limitations. Before we get to those, I’ll talk you through how to use lock.
The following class FileAccessorNoThreadSafety has a single method called DoOperation. It writes a FileStream to disk, writes text to it and then reads it in again.

public class FileAccessorNoThreadSafety
{
	private static int _count;

	public void DoOperation()
	{
		try
		{
			var fs = new FileStream(Constants.TextFileName, FileMode.Append, FileAccess.Write);

			using (var sw = new StreamWriter(fs))
			{
				sw.Write("writing line with count {0}", _count);
				Console.WriteLine("writing line with count {0}", _count);
				sw.Flush();
			}

			fs = new FileStream(Constants.TextFileName, FileMode.Open, FileAccess.Read);

			using (var sr = new StreamReader(fs))
			{
				_count++;
				Trace.WriteLine(string.Format("{0}", sr.ReadToEnd()));
			}
		}
		catch (IOException ioException)
		{
			Console.WriteLine("{0} - {1}", ioException.GetType().Name, ioException.Message);
			Console.WriteLine("Execution deliberately ended due to {0}", ioException.GetType().Name);
			Console.WriteLine("Exiting");
			Trace.WriteLine(string.Format("Execution deliberately ended due to {0}", ioException.GetType().Name));
			Trace.WriteLine(string.Format("Exiting {0}", Thread.CurrentThread.ManagedThreadId));

			Environment.Exit(0); // kill the whole process
		}
		catch (Exception exception)
		{
			Console.WriteLine("{0} - {1}", exception.GetType().Name, exception.Message);
			Console.WriteLine("Execution deliberately ended due to {0}", exception.GetType().Name);
			Console.WriteLine("Exiting");
			Trace.WriteLine(string.Format("Execution deliberately ended due to {{0}}{0}", exception.GetType().Name));
			Trace.WriteLine(string.Format("Exiting {0}", Thread.CurrentThread.ManagedThreadId));

			Environment.Exit(0); // kill the whole process
		}
	}
}

Note that the FileStream is in append mode, so every time that method is invoked, the line of text is appended to the end of the existing text in the file.

The following method is the Main method of a console application which spins up 1000 threads, all looking to invoke DoOperation at the same time (relatively):

private static void Main()
{
	EnsureTextFile();
	Trace.WriteLine(string.Format("Main thread {0}", Thread.CurrentThread.ManagedThreadId));

	Thread[] threads = new Thread[1000];
	string oi = string.Empty;

	var fileAccessor = new FileAccessorNoThreadSafety();
	//var fileAccessor = new FileAccessorThreadSafe();

	for (int i = 0; i < threads.Length; i++)
	{
		threads[i] = new Thread(fileAccessor.DoOperation);
	}

	for (int i = 0; i < threads.Length; i++)
	{
		threads[i].Start();
	}

	Console.ReadLine();
}

private static void EnsureTextFile()
{
	if (!File.Exists(Constants.TextFileName))
	{
		using (File.CreateText(Constants.TextFileName))
		{
			//  do nothing
			//  "using" disposes of Stream returned by CreateText method
		}
	}
}

As all of those threads are simultaneously trying to access the same resource (the file written by DoOperation), it very quickly throws an IOException. So, we have code which is not thread-safe that results in an exception, if more than one thread attempts to invoke it. How can we fix that?

The easy way, is with the lock keyword, which I have used in a method of the same name in another class called FileAccessorThreadSafe:

public class FileAccessorThreadSafe
{
	private static int _count;
	private readonly object threadLock = new object();

	public void DoOperation()
	{
		try
		{
			lock (threadLock)
			{
				var fs = new FileStream(Constants.TextFileName, FileMode.Append, FileAccess.Write);

				using (var sw = new StreamWriter(fs))
				{
					sw.WriteLine("writing line with count {0}", _count);
					Console.WriteLine("writing line with count {0}", _count);
					sw.Flush();
				}

				fs = new FileStream(Constants.TextFileName, FileMode.Open, FileAccess.Read);

				using (var sr = new StreamReader(fs))
				{
					_count++;
					Trace.WriteLine(sr.ReadToEnd() + " " + _count);
				}
			}
		}
		catch (IOException ioException)
		{
			Console.WriteLine("{0} - {1}", ioException.GetType().Name, ioException.Message);

		}
		catch (Exception exception)
		{

			Console.WriteLine("{0} - {1}", exception.GetType().Name, exception.Message);
		}
	}
}

You’ll find that every thread is able to successfully complete the DoOperation method. In effect, you have a blocking call on each thread until each thread is able to acquire a lock on the resource. In layman’s terms, each thread waits patiently for the resource to be released until it is able to acquire a lock. That being the case, no 2 threads are working with the resource at the same time. Lets take a closer look.

The lock keyword is actually a shortened version of something like this (using the DoOperation method to demonstrate):

bool acquired = false;
object tmp = threadLock;

Monitor.Enter(tmp, ref acquired);

try
{
	var fs = new FileStream(Constants.TextFileName, FileMode.Append, FileAccess.Write);

	using (var sw = new StreamWriter(fs))
	{
		sw.WriteLine("writing line with count {0}", _count);
		Console.WriteLine("writing line with count {0}", _count);
		sw.Flush();
	}

	fs = new FileStream(Constants.TextFileName, FileMode.Open, FileAccess.Read);

	using (var sr = new StreamReader(fs))
	{
		_count++;
		Trace.WriteLine(sr.ReadToEnd() + " " + _count);
	}
}
finally
{
	if (acquired)
	{
		Monitor.Exit(tmp);
	}
}

The Monitor class is what is actually used to lock down the resource for the use of that particular thread and in a finally block, so long as the lock was successfully acquired, the Monitor releases its hold on the resource.
Thus, if you do need finer grain control, you may want to use that longer notation. You may find yourself in situations where you want to clean things up in such a way that having that full code is required.

The Bad

So what is the problem with lock? One word – deadlocks. Whilst I’ve never experienced a deadlock, smarter people than I have stated that they are possible. To demonstrate a deadlock, I have written a class with the 2 methods, LockIntThenFloat and LockFloatThenInt, which are specifically designed to deadlock each other:

public class TypeLocker
{
	public void LockIntThenFloat()
	{
		lock (typeof(int))
		{
			Console.WriteLine("Thread {0} got its first lock", Thread.CurrentThread.ManagedThreadId);


			Thread.Sleep(500);

			Console.WriteLine("Sleep over for thread {0}", Thread.CurrentThread.ManagedThreadId);

			lock (typeof(float))
			{
				Console.WriteLine("Thread {0} got both locks", Thread.CurrentThread.ManagedThreadId);
			}

		}
	}

	public void LockFloatThenInt()
	{
		lock (typeof(float))
		{
			Console.WriteLine("Thread {0} got its first lock", Thread.CurrentThread.ManagedThreadId);

			Thread.Sleep(500);

			Console.WriteLine("Sleep over for thread {0}", Thread.CurrentThread.ManagedThreadId);

			lock (typeof(int))
			{
				Console.WriteLine("Thread {0} got both locks", Thread.CurrentThread.ManagedThreadId);
			}
		}
	}  
}

To see that in action, the following console application will never exit:

static void Main()
{
	Console.WriteLine("Deadlock means that the app will never exit.{0}", Environment.NewLine);

	TypeLocker typeLockerInt = new TypeLocker();
	TypeLocker typeLockerFloat = new TypeLocker();

	Thread floatLockFirst = new Thread(typeLockerFloat.LockFloatThenInt);
	Thread intLockFirst = new Thread(typeLockerInt.LockIntThenFloat);

	floatLockFirst.Start();
	intLockFirst.Start();

	//  This will never ever exit
}

So, now that we know that deadlocks are possible and have actually seen one in action, what does the framework give us to solve the problem that sees the lock keyword come a cropper?

The Enhanced

A couple of really switched on guys tackled this issue and put their work on Github.
We can now modify our code in the following way and avoid deadlocks by using timeouts:

public class TypeLockerWithTimeout
{
	public void LockIntThenFloat()
	{
		lock (typeof (int))
		{
			Console.WriteLine("Thread {0} got its first lock", Thread.CurrentThread.ManagedThreadId);

			Thread.Sleep(500);
		}

		Console.WriteLine("Sleep over for thread {0}", Thread.CurrentThread.ManagedThreadId);

		try
		{
			TimedLock timedLock = TimedLock.Lock(typeof (float), TimeSpan.FromSeconds(2));

			Console.WriteLine("Thread {0} got both locks", Thread.CurrentThread.ManagedThreadId);

			timedLock.Dispose();
		}
		catch (LockTimeoutException e)
		{
			Console.WriteLine("Couldn't get a lock!");
			StackTrace otherStack = e.GetBlockingStackTrace(5000);
			if (otherStack == null)
			{
				Console.WriteLine("Couldn't get other stack!");
			}
			else
			{
				Console.WriteLine("Stack trace of thread that owns lock!");
			}
		}
	}

	public void LockFloatThenInt()
	{
		lock (typeof (float))
		{
			Console.WriteLine("Thread {0} got its first lock", Thread.CurrentThread.ManagedThreadId);

			Thread.Sleep(500);
		}

		Console.WriteLine("Sleep over for thread {0}", Thread.CurrentThread.ManagedThreadId);

		try
		{
			TimedLock timedLock = TimedLock.Lock(typeof (int), TimeSpan.FromSeconds(2));
			Console.WriteLine("Thread {0} got both locks", Thread.CurrentThread.ManagedThreadId);
			timedLock.Dispose();
		}
		catch (LockTimeoutException e)
		{
			Console.WriteLine("Couldn't get a lock!");
			StackTrace otherStack = e.GetBlockingStackTrace(5000);
			if (otherStack == null)
			{
				Console.WriteLine("Couldn't get other stack!");
			}
			else
			{
				Console.WriteLine("Stack trace of thread that owns lock!");
			}
		}
	}
}

In the methods of that modified class, you can see that I am still using lock to get the first lock. When it comes to locking down that next resource, I employ Haack’s TimedLock class which uses a timeout of 2 seconds. That is enough time to wait for the locked resource (which was locked by the other method) to be released.

This exercise gave me a much better understanding of lock. I hope it does for you as well.

The source code for this article can be downloaded here: