Is it thread-safe to lock a .NET Dictionary on write, and read it lock-free?

Question

I know that dictionaries are not thread safe.

But for sake of simplicity I sometimes (when the scenario is not too complex) wrap them around a lock and used it in multi-thread environment even if other concurrent structure may be used (eg : ConcurrentDictionary).

Here an example of what I am doing :

 public class Example 
 {
     private object _sync = new object();
     private Dictionary<string,stirng> _dictionary = new Dictionary<string,string> ();

     public void Write (string key, stirng value)
     {
          lock(_sync)
          {
                if (_dictionary.ContainsKey(key) == false)
                     _dictionary.Add(key,value);
          }
     }

     public string Read (string key)
     {
          if (_dictionary.ContainsKey(key))
              return _dictionary[key];

          return null;
     }
}

I am afraid that maybe even reading the dictionary while writing, may produce inconsistent data.
I am not scared about reading old data, and not "seeing" the new one (till the write end) that's ok in my scenario.
Even if the new data is the only inconsistent one (till the write operation end) it's ok.
What I am concerned about is that maybe the dictionary structure is in a usable state while inserting (and so a read in that moment will produce a complete messed up result even for very old key and not only the new one).

Is there any reason you can't use a `ConcurrentDictionary` ? — mathis1337, Aug 21 '22 at 23:39
@mathis1337 ConcurrentDictionaries are really more complex and i am not very familiar with them (nor people i work with). So if the project is not too complex (one or two dictionary in "cirtical" point, with low usage) i prefer to avoid ConcurrentDictionary. So feel to me that the code will be simpler to read and to understand. But lately i start having doubt about correctenss of my usage (regardless of best-practice, that i know i am ignoring to a certain degree). — Skary, Aug 21 '22 at 23:51
"maybe the dictionary structure is in a usable state while inserting". Definitely possible. For example, the buckets may have to be reallocated, and the reading thread may read the old buckets but the new bucket size, resulting it not finding a present element or an array index out of bounds exception. — Raymond Chen, Aug 22 '22 at 00:34
Yes this can absolutely go badly wrong. You need a `lock` on the read also. — Charlieface, Aug 22 '22 at 00:51
Can you please clarify how you come up with that code after reading https://learn.microsoft.com/en-us/dotnet/api/system.collections.generic.dictionary-2?view=net-6.0#thread-safety? (". In the rare case where an enumeration contends with write accesses, the collection must be locked during the entire enumeration. "). You clearly have some wrong ideas about synchronization, but it is unclear what those are exactly. — Alexei Levenkov, Aug 22 '22 at 00:56
Here is a simple repro of it going pear-shaped https://dotnetfiddle.net/o7fzbp — Charlieface, Aug 22 '22 at 00:57
@AlexeiLevenkov this isn't enumeration however. This is hashcode-based lookups. It doesn't make the access any more safe of course. — David L, Aug 22 '22 at 00:57
@DavidL it's *just* hash lookup only in fantasy land to my knowledge :) All practical dictionary/hashmap implementation I know need to do some sort of enumeration on hash collision. — Alexei Levenkov, Aug 22 '22 at 01:00
@AlexeiLevenkov that's a fair point. I'm not convinced that is what the docs are trying to say, but you're absolutely right about the internals. — David L, Aug 22 '22 at 01:02
@DavidL I think both of you are misunderstanding the point of that line of the docs. It's a catch-all, the same text is present in the [SortedList docs](https://learn.microsoft.com/en-us/dotnet/api/system.collections.generic.sortedlist-2.system-collections-icollection-issynchronized?view=net-6.0#remarks) and it's just trying to say "read the collection" as opposed to writing. — Charlieface, Aug 22 '22 at 01:04
@AlexeiLevenkov when people talk about enumeration, they mean doing a `foreach` loop or using an `IEnumerator`. They don't refer to the internal implementation of a data structure, that may involve walking a linked-list to resolve hash-collisions. — Theodor Zoulias, Aug 22 '22 at 05:06

Theodor Zoulias · Accepted Answer · 2022-08-22T05:38:35.013

3

No, it's not thread-safe. The Dictionary<TKey,TValue> is thread-safe for multiple readers, only if it's in a permanent immutable ("frozen") state. If you want to read and write into a dictionary from multiple threads concurrently, then you must lock invariably on all operations (read and write), otherwise the behavior of your program is undefined.

Locking is fast: you can expect to acquire and release a lock in as little as 20 nanoseconds on a 2010-era computer if the lock is uncontended. So you shouldn't worry too much about the performance of your app, when its correctness is at stake.¹

¹ _{Locking is fast, but not without cost. If your app is expected to do millions of read and write operations per second on the same dictionary, switching to a ConcurrentDictionary<K,V> will be most likely significantly beneficial for the performance of your app.}

edited Aug 22 '22 at 05:38

answered Aug 22 '22 at 04:51

Theodor Zoulias

34,835
7
69
104

1

Computers are fast. We think everything they do is fast. Locking is a slow operation, relatively speaking. I think it's a bit misleading to quote a misleading article saying locking is fast. – Enigmativity Aug 22 '22 at 05:03
@Enigmativity my assumption is that the OP has avoided to `lock` when reading the dictionary, because they worry about the performance of their app. They might worry less if they learn that a `lock` can be acquired and released 50,000,000 times per second on a 2010-era computer, if the lock is uncontended. I admit that I am biased by thinking that 50,000,000 times per second is "fast", but the numbers are there for everyone to form their own opinion. – Theodor Zoulias Aug 22 '22 at 05:12
1

It's only fast "if the lock is uncontended" - and if it's not don't use `lock`. But even without contention, `for (int i = 0; i < 50000000; i++) lock (gate) flag = !flag;` is 10x slower with the `lock`. 90ms without and 874ms with. – Enigmativity Aug 22 '22 at 05:20
@Enigmativity I said that the `lock` is fast, not that its cost is zero. If you think that the performance statement in my answer is misleading, would you like me to add a footnote that the statement represents the opinion of the author, and not the unanimous opinion of the whole C# community? – Theodor Zoulias Aug 22 '22 at 05:27
When performance matters it matters. I think a disclaimer needs to state that `lock` hurts performance in performance critical applications, but in general its usage is fast. – Enigmativity Aug 22 '22 at 05:30
3

@Enigmativity I added a footnote. I hope it's better now! – Theodor Zoulias Aug 22 '22 at 05:39
1

Thanks all for the extemely compelte answer and annotation about side effect and issue. No my project is very simple (and not heavely used), i simply overlooked a possibile issue and do an oversemplification. I will definly dig deeper in ConcurrentDictionary usage and switch to them (after i have better understand how all the TrySomething method may fail and how to "roll-back" in a safe situation) – Skary Aug 22 '22 at 07:26
@Skary the `TryX` methods of the `ConcurrentDictionary` are easy to use. The tricky part is the `GetOrAdd` and `AddOrUpdate` methods with lambda parameters. Many people have fallen in the trap of believing that the lambda is invoked in a synchronized fashion, which [is not](https://stackoverflow.com/questions/39156752/is-addorupdate-thread-safe-in-concurrentdictionary) [the case](https://stackoverflow.com/questions/48670104/is-this-thread-safe-with-this-concurrentdictionary-and-addorupdate-method). – Theodor Zoulias Aug 22 '22 at 07:34

Is it thread-safe to lock a .NET Dictionary on write, and read it lock-free?

1 Answers1