CODEWITHSUNDEEP
c#.netcollectionsthread-safety
Guy
Guy

Reputation: 67380

Thread safety with Dictionary<int,int> in .Net

I have this function: 

static Dictionary<int, int> KeyValueDictionary = new Dictionary<int, int>();
static void IncreaseValue(int keyId, int adjustment)
{
    if (!KeyValueDictionary.ContainsKey(keyId))
    {
        KeyValueDictionary.Add(keyId, 0);
    }
    KeyValueDictionary[keyId] += adjustment;
}

Which I would have thought would not be thread safe. However, so far in testing it I have not seen any exceptions when calling it from multiple threads at the same time.

My questions: Is it thread safe or have I just been lucky so far? If it is thread safe then why?

Upvotes: 10

Views: 4542

Answers (4)

jason
jason

Reputation: 241779

However, so far in testing it I have not seen any exceptions when calling it from multiple threads at the same time.

Is it thread safe or have I just been lucky so far? If it is thread safe then why?

You're getting lucky. These types of bugs with threads are so easy to make because testing can you give you a false sense of security that you did things correctly.

It turns out that Dictionary<TKey, TValue> is not thread-safe when you have multiple writers. The documentation explicitly states:

A Dictionary<TKey, TValue> can support multiple readers concurrently, as long as the collection is not modified. Even so, enumerating through a collection is intrinsically not a thread-safe procedure. In the rare case where an enumeration contends with write accesses, the collection must be locked during the entire enumeration. To allow the collection to be accessed by multiple threads for reading and writing, you must implement your own synchronization.

Alternatively, use ConcurrentDictionary. However, you still must write correct code (see note below).

In addition to the lack of thread-safety with Dictionary<TKey, TValue> which you've been lucky to avoid, your code is dangerously flawed. Here's how you can get a bug with your code:

static void IncreaseValue(int keyId, int adjustment) {
    if (!KeyValueDictionary.ContainsKey(keyId)) {
        // A
        KeyValueDictionary.Add(keyId, 0);
    }
    KeyValueDictionary[keyId] += adjustment;
}
  1. Dictionary is empty.
  2. Thread 1 enters the method with keyId = 17. As the Dictionary is empty, the conditional in the if returns true and thread 1 reaches the line of code marked A.
  3. Thread 1 is paused and thread 2 enters the method with keyId = 17. As the Dictionary is empty, the conditional in the if returns true and thread 2 reaches the line of code marked A.
  4. Thread 2 is paused and thread 1 resumes. Now thread 1 adds (17, 0) to the dictionary.
  5. Thread 1 is paused and now thread 2 resumes. Now thread 2 tries to add (17, 0) to the dictionary. An exception is thrown because of a key violation.

There are other scenarios in which an exception can occur. For example, thread 1 could be paused while loading the value of KeyValueDictionary[keyId] (say it loads keyId = 17, and obtains the value 42), thread 2 could come in and modify the value (say it loads keyId = 17, adds the adjustment 27), and now thread 1 resumes and adds its adjustment to the value it loaded (in particular, it doesn't see the modification that thread 2 made to the value associated with keyId = 17!).

Note that even using a ConcurrentDictionary<TKey, TValue> could lead to the above bugs! Your code is NOT safe for reasons not related to the thread-safety or lack thereof for Dictionary<TKey, TValue>.

To get your code to be thread-safe with a concurrent dictionary, you'll have to say:

KeyValueDictionary.AddOrUpdate(keyId, adjustment, (key, value) => value + adjustment);

Here we are using ConcurrentDictionary.AddOrUpdate.

Upvotes: 23

QuinnG
QuinnG

Reputation: 6424

The .NET library has a thread safe dictionary, the ConcurrentDictionary<TKey, TValue> http://msdn.microsoft.com/en-us/library/dd287191.aspx

Updated: I didn't exactly answer the question, so here's updated with more answery to exact question posed. As per the MSDN:http://msdn.microsoft.com/en-us/library/xfhwa508.aspx

A Dictionary can support multiple readers concurrently, as long as the collection is not modified. Even so, enumerating through a collection is intrinsically not a thread-safe procedure. In the rare case where an enumeration contends with write accesses, the collection must be locked during the entire enumeration. To allow the collection to be accessed by multiple threads for reading and writing, you must implement your own synchronization.

For a thread-safe alternative, see ConcurrentDictionary.

Public static (Shared in Visual Basic) members of this type are thread safe.

Upvotes: 2

LukeH
LukeH

Reputation: 269628

You've just been lucky so far. It's not thread-safe.

From the Dictionary<K,V> documentation...

A Dictionary<TKey, TValue> can support multiple readers concurrently, as long as the collection is not modified. Even so, enumerating through a collection is intrinsically not a thread-safe procedure. In the rare case where an enumeration contends with write accesses, the collection must be locked during the entire enumeration. To allow the collection to be accessed by multiple threads for reading and writing, you must implement your own synchronization.

Upvotes: 1

Joshua
Joshua

Reputation: 43327

It's not thread safe, but does not check and so probably doesn't notice silent corruption.

It will appear to be thread safe for a long time because only when it needs to rehash() does it have even a chance of exception. Otherwise, it just corrupts data.

Upvotes: 2

Related Questions