The problem with the BlockingCollection

October 11, 2018

The BlockingCollection is a really powerful thread-safe collection that can be a really powerful tool to use where appropriate. Generally you could consider it where you have a common instance of a class accessed from multiple threads. It gives the class control on how many items are added or taken in a thread-safe way. Calling the Take when the collection is empty or an Add when the collection is full blocks the thread until something is added/removed. So an example is to imagine your class gets data from a wcf client. To improve performance you might want to have multiple shared client instances. A BlockingCollection could really help you there because you could bound it to say max 4 instances of the client contract. Each consuming thread will try take an instance from the collection and add it back once done. If all 4 instances are checked out then the next call will block and wait until there is one available. This is incredibly powerful and it would keep the code maintaining this quite small.

public class BlockingExample
{
    private readonly BlockingCollection _clients;

    public BlockingExample(IClientContractFactory factory)
    {
        int maxClients = 4;
        _clients = new BlockingCollection(maxClients);

        for (int i = 0; i < maxClients; i++)
        {
            _clients.Add(factory.Get());
        } 
    }

    public int DoWork()
    {
        var client = _clients.Take();

        var result = client.DoWork();

        _clients.Add(client);

        return result;
    }

}

[ServiceContract]
public interface IClientContract
{
    int DoWork();
}

public interface IClientContractFactory
{
    IClientContract Get();
}

So this is great and in this case it really simplifies the code. There is one major downside to this though. Testability. How do you test this behavior that you are expecting to happen in production? Is it really a good idea to have this potentially critical class completely untested? Not saying you can't test it exactly but it would be hard to test the behavior without your test method spinning up multiple threads. Adding multiple threads into unit tests and you can potentially have tests that run fine locally on your fast machine but fail once every few times on the build server. Personally when I see Taskfactories/new threads in unit tests I think it points to something too complex to properly test. I'm not saying that this is all bad. Its just that you need to weight the actual cost in using this. You lose testability. When there are bugs, how do you write a test to ensure the bug is properly fixed? Its not trivial.