http://ayende.comAyende @ RahienCopyright (C) Ayende Rahien 2004 - 2021 (c) 202660I missed the last fine-print of Ayende's entry. Ok, I'll agree that in a bad implementation, thread B while executing its outer ContainsKey(o) enumerating can potentially break. http://ayende.com/4823/why-is-this-not-thread-safe#comment15http://ayende.com/4823/why-is-this-not-thread-safe#comment15Thu, 05 May 2011 19:14:50 GMT@Sony, No, it isn't thread safe. If thread A enters the lock and ContainsKey returns false, it will call Add. Now if in the middle of the Add (before it completes), thread B calls ContainsKey on the dictionary, the invariants of ContainsKey can be broken. As Ayende states, this is implementation dependent. But consider, what if Add had finished adding the key (so ContainsKey returns true) but not the value. At this point the indexer could return null or throw an exception if you try to access a key *you believe is already in the dictionary*. Hence, not thread safe. http://ayende.com/4823/why-is-this-not-thread-safe#comment14http://ayende.com/4823/why-is-this-not-thread-safe#comment14Tue, 03 May 2011 00:34:45 GMTThis code is perfectly thread safe and the documentation supplied does not invalidate the thread safety of the code as I see it. Once 'items' is locked successfully by a thread, no modifications can occur, your reads including enumerations and your if-check will be guaranteed. http://ayende.com/4823/why-is-this-not-thread-safe#comment13http://ayende.com/4823/why-is-this-not-thread-safe#comment13Mon, 02 May 2011 18:07:23 GMTConcurrentDictionary is the solution, and has a great API too http://ayende.com/4823/why-is-this-not-thread-safe#comment12http://ayende.com/4823/why-is-this-not-thread-safe#comment12Thu, 28 Apr 2011 20:47:48 GMTThis is a pretty common pitfall. It might show itself as a livelock (very high or 100% CPU in production). http://ayende.com/4823/why-is-this-not-thread-safe#comment11http://ayende.com/4823/why-is-this-not-thread-safe#comment11Wed, 27 Apr 2011 11:57:55 GMT@configurator you might want to consider that it is still several times slower to acquire and release a reader writer lock than a standard lock. If the thing you are doing inside the lock is fast, a simple lock should be fine. You can test it out to see what performance gain you would actually get, my guess is hardly any. http://ayende.com/4823/why-is-this-not-thread-safe#comment10http://ayende.com/4823/why-is-this-not-thread-safe#comment10Wed, 27 Apr 2011 11:55:54 GMTThe example code is not thread-safe as the data structure is mutable, and an unlocked read operation is allowed. At any given time other thread(s) may be adding new items to the collection and therefore mutating it. If ContainsKey is executing whist the collection is being mutated, the outcome is indeterminate. For instance ContainsKey may depend on intermediate results as it executes which could become invalid as the collection is mutated by the addition of a new item e.g. the collection may have to be re-arranged to maintain the load factor when a new item is added. This could produce an incorrect result or an exception during the execution of ContainsKey. http://ayende.com/4823/why-is-this-not-thread-safe#comment9http://ayende.com/4823/why-is-this-not-thread-safe#comment9Wed, 27 Apr 2011 08:29:28 GMTA ReaderWriterLockSlim would be a good solution here. ConcurrentDictionary is sometimes clunky, it all depends on the rest of the using code; in this example, it would be easy to replace it with TryAdd, but if anyone else uses this dictionary they'd need to change their code. Of course, with a ReaderWriterLock they'd need to acquire an appropriate lock, but it's much easier to avoid issues that way sometimes. http://ayende.com/4823/why-is-this-not-thread-safe#comment8http://ayende.com/4823/why-is-this-not-thread-safe#comment8Tue, 26 Apr 2011 18:16:50 GMT@Eber, no it's not. The ContainsKey call outside of the lock is not thread safe. http://ayende.com/4823/why-is-this-not-thread-safe#comment7http://ayende.com/4823/why-is-this-not-thread-safe#comment7Tue, 26 Apr 2011 18:06:28 GMTSo this code *is* thread safe, the code that would not be thread safe would be the one enumerating the collection http://ayende.com/4823/why-is-this-not-thread-safe#comment6http://ayende.com/4823/why-is-this-not-thread-safe#comment6Tue, 26 Apr 2011 14:41:08 GMT@Alex, that's the most pessimistic approach though. http://ayende.com/4823/why-is-this-not-thread-safe#comment5http://ayende.com/4823/why-is-this-not-thread-safe#comment5Tue, 26 Apr 2011 13:49:39 GMTJust inner part works fine: lock(items) { if(items.ContainsKey(key) == false) { items.Add(key, val); } } http://ayende.com/4823/why-is-this-not-thread-safe#comment4http://ayende.com/4823/why-is-this-not-thread-safe#comment4Tue, 26 Apr 2011 13:31:13 GMT@John the "best" solution is to use ConcurrentDictionary. [msdn.microsoft.com/en-us/library/dd287191.aspx](http://msdn.microsoft.com/en-us/library/dd287191.aspx)http://ayende.com/4823/why-is-this-not-thread-safe#comment3http://ayende.com/4823/why-is-this-not-thread-safe#comment3Tue, 26 Apr 2011 13:19:42 GMTso what's the solution? could you use a resetEvent to only block when writing? http://ayende.com/4823/why-is-this-not-thread-safe#comment2http://ayende.com/4823/why-is-this-not-thread-safe#comment2Tue, 26 Apr 2011 13:09:41 GMTFor example during an array expansion. http://ayende.com/4823/why-is-this-not-thread-safe#comment1http://ayende.com/4823/why-is-this-not-thread-safe#comment1Tue, 26 Apr 2011 09:20:48 GMT