Unit test for thread safe-ness?

I've written a class and many unit test, but I did not make it thread safe. Now, I want to make the class thread safe, but to prove it and use TDD, I want to write some failing unit tests before I start refactoring.

Any good way to do this?

My first thought is just create a couple threads and make them all use the class in an unsafe way. Do this enough times with enough threads and I'm bound to see it break.

There are two products that can help you there:

• Microsoft Chess
• Typemock Racer

Both check for deadlocks in your code (via unit test) and I think Chess checks for race conditions as well.

Using both tools is easy - you write a simple unit test and the run your code several times and check if deadlocks/race conditions is possible in your code.

Edit: Google has released a tool that checks for race condition in runtime (not during tests) that called thread-race-test.
it won't find all of the race conditions because it only analyse the current run and not all of the possible scenarios like the tool above but it might help you find the race condition once it happens.

Update: Typemock site no longer had a link to Racer, and it was not been updated in the last 4 years. I guess the project was closed.

The problem is that most of the multi-threading issues, like race conditions, are non-deterministic by their nature. They can depend on hardware behavior which you can't possibly emulate or trigger.

That means, even if you make tests with multiple threads, they will not be consistently failing if you have a defect in your code.

Note that Dror's answer does not explicitly say this, but at least Chess (and probably Racer) work by running a set of threads through all their possible interleavings to get repreoducible errors. They do not just run the threads for a while hoping that if there is an error it will happen by coincidence.

Chess for example will run through all interleavings and then give you a tag string that represents the interleaving that a deadlock was found on so that you can attribute your tests with the specific interleavings that are interesting from a deadlocking perspective.

I do not know the exact internal workings of this tool, and how it maps these tag strings back to code that you may be changing to fix a deadlock, but there you have it... I am actually really looking forward to this tool (and Pex) becoming part of the VS IDE.

I have seen people try to test this with standard unittests as you yourself propose. The tests are slow, and have so far failed to identify a single of the concurrency problems our company struggles with.

After many failures, and despite my love for unittests, I have come to accept that errors in concurrency is not one of unittests strengths. I usually encourage analysis and review in favour of unittests for classes where concurrency is a subject. With total overview of the system it is in many cases possible to prove/falsify claims of thread safety.

Anyhow I would love for someone to give me something that might point to the opposite, so I watch this question closely.

When I recently had to address the same problem I thought of it this way; First of all your existing class has one responsibility and that is to provide some functionality. It is not the objects responsibility to be thread safe. If it needs to be thread safe some other object should be used to provide this functionality. But if some other object is providing the thread safe-ness it cannot be optional because then you cannot prove your code is thread safe. So this is how I handle it:

// This interface is optional, but is probably a good idea.
public interface ImportantFacade
{
    void ImportantMethodThatMustBeThreadSafe();
}

// This class provides the thread safe-ness (see usage below).
public class ImportantTransaction : IDisposable
{
    public ImportantFacade Facade { get; private set; }
    private readonly Lock _lock;

    public ImportantTransaction(ImportantFacade facade, Lock aLock)
    {
        Facade = facade;
        _lock = aLock;
        _lock.Lock();
    }

    public void Dispose()
    {
        _lock.Unlock();
    }
}

// I create a lock interface to be able to fake locks in my tests.
public interface Lock
{
    void Lock();
    void Unlock();
}

// This is the implementation I want in my production code for Lock.
public class LockWithMutex : Lock
{
    private Mutex _mutex;

    public LockWithMutex()
    {
        _mutex = new Mutex(false);
    }

    public void Lock()
    {
        _mutex.WaitOne();
    }

    public void Unlock()
    {
        _mutex.ReleaseMutex();
    }
}

// This is the transaction provider. This one should replace all your
// instances of ImportantImplementation in your code today.
public class ImportantProvider<T> where T:Lock,new()
{
    private ImportantFacade _facade;
    private Lock _lock;

    public ImportantProvider(ImportantFacade facade)
    {
        _facade = facade;
        _lock = new T();
    }

    public ImportantTransaction CreateTransaction()
    {
        return new ImportantTransaction(_facade, _lock);
    }
}

// This is your old class.
internal class ImportantImplementation : ImportantFacade
{
    public void ImportantMethodThatMustBeThreadSafe()
    {
        // Do things
    }
}

The use of generics makes it possible to use a fake lock in your tests to verify that the lock is always taken when a transaction is created and not released until transaction is disposed. Now you can also verify that the lock is taken when your important method is called. Usage in production code should look something like this:

// Make sure this is the only way to create ImportantImplementation.
// Consider making ImportantImplementation an internal class of the provider.
ImportantProvider<LockWithMutex> provider = 
    new ImportantProvider<LockWithMutex>(new ImportantImplementation());

// Create a transaction that will be disposed when no longer used.
using (ImportantTransaction transaction = provider.CreateTransaction())
{
    // Access your object thread safe.
    transaction.Facade.ImportantMethodThatMustBeThreadSafe();
}

By making sure the ImportantImplementation cannot be created by somebody else (by for example create it in the provider and make it a private class) you kan now prove your class is thread safe since it cannot be accessed without a transaction and the transaction always takes the lock when created and releases it when disposed.

Make sure the transaction is disposed correctly can be harder and if not you might see weird behaviour in your application. You can use tools as Microsoft Chess (as suggested in another anser) to look for things like that. Or you can have your provider implement the facade and make it implement it like this:

    public void ImportantMethodThatMustBeThreadSafe()
    {
        using (ImportantTransaction transaction = CreateTransaction())
        {
            transaction.Facade.ImportantMethodThatMustBeThreadSafe();
        }
    }

Even though this is the implementation I hope you can figure out the tests to verify these classes as needed.

testNG or Junit with springframeworks test module (or other extension) has basic support for concurrency testing.

This link might interest you

http://www.cs.rice.edu/~javaplt/papers/pppj2009.pdf

you'll have to construct a test case for each concurrency scenario of concern; this may require replacing efficient operations with slower equivalents (or mocks) and running multiple tests in loops, to increase the chance of contentions

without specific test cases, it is difficult to propose specific tests

some potentially useful reference material:

• Oren Ellenbogen's Blog
• Vertical Slice Blog
• possible duplicate SO question

Though it's not as elegant as using a tool like Racer or Chess, I have used this sort of thing for testing for thread safety:

// from linqpad

void Main()
{
	var duration = TimeSpan.FromSeconds(5);
	var td = new ThreadDangerous();	
	
	// no problems using single thread (run this for as long as you want)
	foreach (var x in Until(duration))
		td.DoSomething();
		
	// thread dangerous - it won't take long at all for this to blow up
	try
	{	        
		Parallel.ForEach(WhileTrue(), x => 
			td.DoSomething());
			
		throw new Exception("A ThreadDangerException should have been thrown");
	}
	catch(AggregateException aex)
	{
		// make sure that the exception thrown was related
		// to thread danger
		foreach (var ex in aex.Flatten().InnerExceptions)
		{
			if (!(ex is ThreadDangerException))
				throw;
		}
	}
	
	// no problems using multiple threads (run this for as long as you want)
	var ts = new ThreadSafe();
	Parallel.ForEach(Until(duration), x => 
		ts.DoSomething());		
		
}

class ThreadDangerous
{
	private Guid test;
	private readonly Guid ctrl;
	
	public void DoSomething()
	{			
		test = Guid.NewGuid();
		test = ctrl;		
		
		if (test != ctrl)
			throw new ThreadDangerException();
	}
}

class ThreadSafe
{
	private Guid test;
	private readonly Guid ctrl;
	private readonly object _lock = new Object();
	
	public void DoSomething()
	{	
		lock(_lock)
		{
			test = Guid.NewGuid();
			test = ctrl;		
			
			if (test != ctrl)
				throw new ThreadDangerException();
		}
	}
}

class ThreadDangerException : Exception 
{
	public ThreadDangerException() : base("Not thread safe") { }
}

IEnumerable<ulong> Until(TimeSpan duration)
{
	var until = DateTime.Now.Add(duration);
	ulong i = 0;
	while (DateTime.Now < until)
	{
		yield return i++;
	}
}

IEnumerable<ulong> WhileTrue()
{
	ulong i = 0;
	while (true)
	{
		yield return i++;
	}
}

The theory is that if you can cause a thread dangerous condition consistently to occur in a very short amount of time, you should be able to bring about thread safe conditions and verify them by waiting a relatively large amount of time without observing state corruption.

I do admit that this may be a primitive way of going about it and may not help in complex scenarios.

Here's my approach. This test is not concerned with deadlocks, it's concerned with consistency. I'm testing a method with a synchronized block, with code that looks something like this:

synchronized(this) {
  int size = myList.size();
  // do something that needs "size" to be correct,
  // but which will change the size at the end.
  ...
}

It's tough to produce a scenario that will reliably produce a thread conflict, but here's what I did.

First, my unit test created 50 threads, launched them all at the same time, and had them all call my method. I use a CountDown Latch to start them all at the same time:

CountDownLatch latch = new CountDownLatch(1);
for (int i=0; i<50; ++i) {
  Runnable runner = new Runnable() {
    latch.await(); // actually, surround this with try/catch InterruptedException
    testMethod();
  }
  new Thread(runner, "Test Thread " +ii).start(); // I always name my threads.
}
// all threads are now waiting on the latch.
latch.countDown(); // release the latch
// all threads are now running the test method at the same time.

This may or may not produce a conflict. My testMethod() should be capable of throwing an exception if a conflict occurs. But we can't yet be sure that this will generate a conflict. So we don't know if the test is valid. So here's the trick: Comment out your synchronized keyword(s) and run the test. If this produces a conflict, the test will fail. If it fails without the synchronized keyword, your test is valid.

That's what I did, and my test didn't fail, so it was not (yet) a valid test. But I was able to reliably produce a failure by putting the code above inside a loop, and running it 100 times consecutively. So I call the method 5000 times. (Yes, this will produce a slow test. Don't worry about it. Your customers won't be bothered by this, so you shouldn't either.)

Once I put this code inside an outer loop, I was able to reliably see a failure on about the 20th iteration of the outer loop. Now I was confident the test was valid, and I restored the synchronized keywords to run the actual test. (It worked.)

You may discover that the test is valid on one machine and not on another. If the test is valid on one machine and your methods pass the test, then it's presumably thread-safe on all machines. But you should test for validity on the machine that runs your nightly unit tests.