Why Dictionary indexer is not thread safe and how to make it

If you read my blog about why ++ operator is not thread safe you’ll know that a one line C# code doesn’t mean it is thread safe and Dictionary indexer is no exception.

Recently someone came to me and ask me: I have a static dictionary in a web application, do you think it is possible that two request try to add the same element at the same time and fails. The answer is obviously yes. Let’s see how this happens.

We will buil a sample code that wil prove the point. Of course this is not real production code but it will be easy to expose the problem. First we need a dictionary:

internal static Dictionary<string, string>() dict = new Dictionary<string, string>();

Let say, somewhere in your code you do something like this:

internal static Dictionary<string, string>() dict = new Dictionary<string, string>();

This single line of code looks better than:

if (!dict.ContainsKey(key))
    dict.Add(key, value);

but it is not.

In both cases you can encounter race condition. Take a look at how the indexer is implemented:

public TValue this[TKey key]
{
    get
    {
        int index = this.FindEntry(key);
        if (index >= 0)
        {
            return this.entries[index].value;
        }
        ThrowHelper.ThrowKeyNotFoundException();
        return default(TValue);
    }
    set
    {
        this.Insert(key, value, false);
    }
}

Ok the setter uses a private Insert method. This is where it gets worst:

private void Insert(TKey key, TValue value, bool add)
{
    int freeList;
    if (key == null)
    {
        ThrowHelper.ThrowArgumentNullException(ExceptionArgument.key);
    }
    if (this.buckets == null)
    {
        this.Initialize(0);
    }
    int num = this.comparer.GetHashCode(key) & 0x7fffffff;
    int index = num % this.buckets.Length;
    for (int i = this.buckets[index]; i >= 0; i = this.entries[i].next)
    {
        if ((this.entries[i].hashCode == num) && this.comparer.Equals(this.entries[i].key, key))
        {
            if (add)
            {
                ThrowHelper.ThrowArgumentException(ExceptionResource.Argument_AddingDuplicate);
            }
            this.entries[i].value = value;
            this.version++;
            return;
        }
    }
    if (this.freeCount > 0)
    {
        freeList = this.freeList;
        this.freeList = this.entries[freeList].next;
        this.freeCount--;
    }
    else
    {
        if (this.count == this.entries.Length)
        {
            this.Resize();
            index = num % this.buckets.Length;
        }
        freeList = this.count;
        this.count++;
    }
    this.entries[freeList].hashCode = num;
    this.entries[freeList].next = this.buckets[index];
    this.entries[freeList].key = key;
    this.entries[freeList].value = value;
    this.buckets[index] = freeList;
    this.version++;
}

That’s alot of code to add an item to the dictionary. As we saw earlier, there’s a lot of “not thread safe” code in there.

The easiest way to resolve this problem is to put a lock around the dictionary operations. First we need an object to lock on. It’s always a good practice to have a separate object to lock on even though it is possible to lock the dictionary itself. We can create that object ourself but every collection already have a SyncLock object but it’s not visible because it’s explicitly implemented. To access it we need to cast the dictionary to an ICollection.

lock (((ICollection)dict).SyncRoot)
{
    dict[key] = value;
}

With this new implementation, our code should run without any problem.