Should I unit test for multithreading problems before writing any lock? (.NET C# TDD)

Question

I am writing a class that I know that needs a lock, because the class must be thread safe. But as I am Test-Driven-Developing I know that I can't write a line of code before creating a test for it. And I find very difficult to do since the tests will get very complex in the end. What do you usually do in those cases? There is any tool to help with that?

This question is .NET specific

Someone asked for the code:

public class StackQueue
{
    private Stack<WebRequestInfo> stack = new Stack<WebRequestInfo>();
    private Queue<WebRequestInfo> queue = new Queue<WebRequestInfo>();

    public int Count
    {
        get
        {
            return this.queue.Count + this.stack.Count;
        }
    }

    public void Enqueue(WebRequestInfo requestInfo)
    {
        this.queue.Enqueue(requestInfo);
    }

    public void Push(WebRequestInfo requestInfo)
    {
        this.stack.Push(requestInfo);
    }

    private WebRequestInfo Next()
    {
        if (stack.Count > 0)
        {
            return stack.Pop();
        }
        else if (queue.Count > 0)
        {
            return queue.Dequeue();
        }
        return null;
    }
}

Answer 1

Just to be clear, not all classes need locks to be thread safe.

If your tests end up being overly complex, it may be a symptom of your classes or methods being too complicated, being two tightly coupled, or taking on too much responsibility. Try to follow the single responsibility principle.

Would you mind posting more concrete information about your class?

Answer 2

Well, you can usually use things like ManualResetEvent to get a few threads into an expected problem state before releasing the gate... but that only covers a small subset of threading issues.

For the bigger problem of threading bugs, there is CHESS (in progress) - maybe an option in the future.

Answer 3

You shouldn't really test for thread safety in a unit test. You should probably have a separate set of stress tests for thread-safety.

---

Okay, now that you've posted the code:

public int Count
    {
        get
        {
            return this.queue.Count + this.stack.Count;
        }
    }

This is a great example of where you're going to have trouble writing a unit test that will expose the threading issues in your code. This code potentially needs synchronization, because the values of this.queue.Count and this.stack.Count can change in the middle of the calculation of the total, so it can return a value that's not "correct".

HOWEVER - Given the rest of the class definition, nothing actually depends on Count giving a consistent result, so does it actually matter if it's "wrong"? There's no way to know that without knowing how other classes in your program use this one. That makes testing for threading issues an integration test, rather than a unit test.

Answer 4

Multithreading can result in so complex problems that it is next to impossible to write unit tests for them. You might manage to write a unit test which has 100% failure rate when executed on a code but after you make it pass, it's quite likely that there are still race conditions and similar problems in the code.

The problem with threading problems is that they crop up randomly. Even if you get a unit test to pass, it doesn't necessarily mean that the code works. So in this case TDD gives a false sense of security and might even be considered a bad thing.

And it is also worth remembering that if a class is thread safe you can use it from several threads without problems - but if a class is not thread safe it doesn't immediately imply that you can't use it from several threads without problems. It still could be thread safe in practice but no one just wants to take responsibility for it not being thread safe. And if it's thread safe in practice, it's impossible to write a unit test which fails because of multi-threading. (Of course most non-thread safe classes really aren't thread safe and will fail happily.)

Answer 5

TDD is a tool -- and a good one -- but sometimes you have problems that are not well-solved using a particular tool. I would suggest that, if developing the tests is overly complex, you should use TDD to develop the expected functionality, but perhaps rely on code inspection to assure your self that the locking code that you add is appropriate and will allow your class to be thread-safe.

One potential solution would be to develop the code that would go inside the lock and put it in its own method. Then you might be able to fake this method to test your locking code. In your fake code you could simply establish a wait to ensure that the second thread accessing the code must have waited on the lock until the first thread completes. Without knowing exactly what your code does, I can't be more specific than that.

Answer 6

Especially with multi-core systems, you can usually test for threading issues, just not deterministically.

The way I usually do it is spin up multiple threads that blast through the code in question and count the number of unexpected results. Those threads run for some short period of time (usually 2-3 seconds), then use Interlocked.CompareExchange to add their results, and exit normally. My test that spun them up then calls .Join on each, then checks to see whether the number of errors was 0.

Sure, it's not foolproof, but with a multi-core CPU, it usually does the job well enough to demonstrate to my coworkers that there's a problem that needs to be addressed with the object (assuming the intended use calls for multi-threaded access).

Answer 7

When writing multi-threaded code, you must use your brain even more than the usual. You must reason logically about every single line of code, whether it is thread safe or not. It's like proving the correctness of a mathematical formula - you can not prove things like "N + 1 > N for all N" by just giving examples of values of N with which the formula is true. Similarly, proving that a class is thread-safe is not possible by writing test cases that try to expose problems with it. With a test it's only possible to prove that there is a fault, but not that there are no faults.

The best thing that you can do, is to minimize the need for multi-threaded code. Preferably the application should have no multi-threaded code (for example by relying on thread-safe libraries and suitable design patterns), or it should be restricted to a very small area. Your StackQueue class looks like simple enough, so that you can make it safely thread-safe with a little thinking.

Assuming that the Stack and Queue implementations are thread-safe (I don't know .NET's libraries), you just need to make Next() thread-safe. Count is already thread-safe as it is, because no client can use the value returned from it safely without using client-based locking - state dependencies between methods would otherwise break the code.

Next() is not thread-safe, because it has state dependencies between methods. If threads T1 and T2 call stack.Count at the same time and it returns 1, then one of them will get the value with stack.Pop(), but the other will call stack.Pop() when the stack is empty (which then appears to throw InvalidOperationException). You will need a stack and queue with non-blocking versions of Pop() and Dequeue() (ones that return null when empty). Then the code would be thread-safe when written like this:

private WebRequestInfo Next()
{
    WebRequestInfo next = stack.PopOrNull()
    if (next == null)
    {
        next = queue.DequeueOrNull();
    }
    return next;
}

Answer 8

Most unit tests operate sequentially and not concurrently, so they will not expose concurrency problems.

Code inspection by programmers with concurrency expertise is your best bet.

Those evil concurrency problems often won't show up until you have enough product in the field to generate some statistically relevant trends. They are incredibly hard to find sometimes, so it is often best to avoid having to write the code in the first place. Use pre-existing thread-safe libraries and well-established design patterns if possible.

Answer 9

I agree with the other posters that multithreaded code should be avoided, or at least confined to small portions of your application. However, I still wanted some way to test those small portions. I'm working on a .NET port of the MultithreadedTC Java library. My port is called Ticking Test, and the source code is published on Google Code.

MultithreadedTC lets you write a test class with several methods marked with a TestThread attribute. Each thread can wait for a certain tick count to arrive, or assert what it thinks the current tick count should be. When all current threads are blocked, a coordinator thread advances the tick count and wakes up any threads that are waiting for the next tick count. If you're interested, check out the MultithreadedTC overview for examples. MultithreadedTC was written by some of the same people that wrote FindBugs.

I've used my port successfully on a small project. The major missing feature is that I have no way to track newly created threads during the test.