Thread-safe code

Consider this code:

if (!dictionary.ContainsKey(key)) {
  dictionary.Add(key, value);
}

In a multithreaded environment, thread A and B may check for the existence of the key at the same time and both determine that the key doesn’t exist. Then, they both try to add it. One of them gets the ArgumentException about the key already been there, because the other thread just added it.

	Thread A	Thread B
1	ContainsKey (returns false)	ContainsKey (returns false)
2	Add	Add

.NET collections are not synchronized as you know. So the above table shows the two threads happily running in parallel.

What I saw in my project’s code is that in some cases the problem was “solved” by using a ThreadSafeDictionary in place of the standard .NET Dictionary class. This custom ThreadSafeDictionary class has synchronized methods. However, this doesn’t actually solve the problem. Consider these possible execution paths:

Thread B gets exception:

	Thread A	Thread B
1	ContainsKey (returns false)	Blocked by Thread A
2	Waits for system to resume Thread A	ContainsKey (return false)
3	Add (ok)	Blocked by Thread A
4		Add (exception)

No exception scenario (if you’re lucky):

	Thread A	Thread B
1	ContainsKey (returns false)	Blocked by Thread A
2	Add (ok)	ContainsKey (return true)

Thread A gets exception:

	Thread A	Thread B
1	ContainsKey (returns false)	Blocked by Thread A
2	Waits for system to resume Thread A	ContainsKey (return false)
3	Blocked by Thread B	Add (ok)
4	Add (exception)

Another one:

	Thread A	Thread B
1	ContainsKey (returns false)	Blocked by Thread A
2	Add (ok)	ContainsKey (return false because Add of Thread A hasn’t quite finished yet)
3		Add (exception)

So the problem is not solved. Why? While the ContainsKey and Add methods of the ThreadSafeDictionary are thread-safe on their own,  the business logic that they create combined isn’t. What you need to do, is to lock your entire block of code that needs to be ran by one thread at a time. This is called the Critical Section. The above code should be written like:

lock (synchronizeObject) {
  if (!dictionary.ContainsKey(key)) {
    dictionary.Add(key, value);
  }
}

This way you lock the entire code block for single thread access and you ensure your data integrity. And, the critical section should be as big as necessary but not more.

The issue becomes more serious for structures that have a long life time (e.g. application scope), which increases the chances of failure over time.