If you are a happy user of Rhino Mocks like I am, you might have experienced the same issue that I noticed this week at work.

We recently ported our entire code base to TFS, and decided to change the unit testing framework to MSTest so we could get a better integration with TFS.

We were a bit unhappy with the build times, so I started looking into running tests in parallel.

Luckily its very easy with MSTest, you simply add a parameter to your Local.testsettings file and you can run up to 5 tests in parallel, which is very nice, since most of us do in fact have multi core processors.

But when we changed the setting, we started seeing random test errors, one test run two tests were failing, another test run they were fine, and every sinle time you ran the test alone they worked perfectly. 

So we figured out it was caused by running multiple tests in the same test fixture in parallel, in conjunction with Rhino Mocks.

At first we thought it was MSTest that somehow was sharing state between tests in the same test fixture, but after a few test we found out that it was in fact Rhino Mocks that was not being built thread safe.

There were two issues, first there was a race condition that happens when you create a stub or mock.

Internally Rhino keeps a dictionary of Type and a proxy generator, so multiple calls to create the same type will have speed benefit. Unfortunately the code was not written in a thread safe way:

if (!generatorMap.ContainsKey(type))
{
    generatorMap[type] = new ProxyGenerator();
}
return generatorMap[type];

So what happens when two threads access this piece of code is that there is a high chance that they both will enter the body of the if statement and try to add the same key to the dictionary and thus fail. This is what we were seeing.

The generatorMap variable is a static variable in the class MockRepository, and as such thats fine, but since multiple threads can access it, there needs to be a guard in place to prevent both threads from trying to add the same key to the dictionary.

I created a small patch for this, not by adding a lock statement, but by simply adding a [ThreadStatic] attribute to the generatorMap variable. 

[ThreadStatic]
private static IDictionary<Type, ProxyGenerator> generatorMap;

For those that do not know what thread static means, let me explain it briefly. TheadStatic means that each thread will get its own instance of the variable, and in that way remove the race condition, since only one thread will access the variable at the same time. The variable will still be static, so several calls to MockRepository will still benefit from the map, but multiple threads will not muck things up for each other. 

One caveat with ThreadStatic is that each thread will have to initialize the member variable, otherwith it will only be the first thread that will get an instance that is not null, so I added a call in the constructor to instantiate the variable if it was not null.

Okay, one problem solved.

Another came up. 

Apparently there was another static variable causing issues. MockRepository holds a reference to the current Repository, which is bad, since it will prevent multi threading from working all together, since multiple threads will change the behaviour of each others mocks and cause tests to fail in the weirdest way. Luckily that was an easy fix, I just added a ThreadStatic to the variable and presto everything started working as expected.

[ThreadStatic]
internal static MockRepository lastRepository;

Luckily for you you do not have to do the same I just did. 

I downloaded the code from the source repository at git, and fixed the code.

I have put it up for download here. 

Just build the code using the instructions in the "How to build.txt" file, and you will get a nice Thread Safe Rhino Mocks dll.

ayende-rhino-mocks-0f0f055.zip (9.93 mb)

Some might ask why I did not submit a patch to the author himself, and I did not do that because it seems like he have abandoned the project entirely. No code has been checked into the project in over a year.