27

I have a static readonly dictionary. No modifications will be made to this dictionary.

I have multiple threads reading from this dictionary using the .ContainsKey(Key). e.g.

class MyData
{ 
    private static private IDictionary<int, string> _dictionary = new Dictionary<int, string>();

    MyData()
    {
        // Load Dictionary here
    }

    public string GetValue(int key)
    {
        if (_dictionary.ContainsKey(key))
        { 
            return _dictionary[key];   
        }
    }
}

Are there any threading problems with doing this?

abatishchev
  • 95,331
  • 80
  • 293
  • 426
Mark 909
  • 1,825
  • 1
  • 17
  • 27

4 Answers4

21

If nobody is mutating it: this is fine. If there were occasional edits, then perhaps look at ReaderWriterLockSlim, or (my preference) edit a snapshot/copy and swap the reference.

Marc Gravell
  • 976,458
  • 251
  • 2,474
  • 2,830
  • But you would still need the ReaderWriterLockSlim whilst swapping the reference, right? – Mark 909 Nov 26 '10 at 16:46
  • 1
    @Mark: Writing a reference is guaranteed to be atomic, but I guess you'd probably want to mark the field `volatile` to ensure that any other threads see the change. – LukeH Nov 26 '10 at 17:08
  • I guess I was thinking that if you swap out the reference whilst it's in the middle of running the ContainsKey method then it might cause problems? – Mark 909 Nov 26 '10 at 17:09
  • 1
    @Mark: You should aim to use `TryGetValue` rather than `dict.ContainsKey` followed by `dict[key]`. If you do that then the old ref will be read *once* atomically and you'll get your value from the old copy of the dictionary. – LukeH Nov 26 '10 at 17:13
  • Sounds great. But just leaves me confused about the purpose of locks given you can just use the edit / swap reference pattern to make updates – Mark 909 Nov 26 '10 at 17:22
  • 2
    @Mark - with TryGetValue you only dereference once, so the behaviour is well-defined (it works); if you do a separate ContainsKey then fetch, you need to *either* copy the reference into a local variable, else risk the second dereference being a different dictionary with different data. – Marc Gravell Nov 27 '10 at 20:38
5

It is safe if you are only going to read.

Darin Dimitrov
  • 994,864
  • 265
  • 3,241
  • 2,902
2

if all of the 'adding' is complete before you read from multiple threads then it is fine. Just because its readonly doesnt mean its thread safe - which it isnt.

Maybe you should user ReaderWriterLock to synchronise access

Dean Chalk
  • 19,361
  • 6
  • 58
  • 86
2

If you were to be writing data at the same time (and you were using .NET 4.0) then you could use the ConcurrentDictionary

Ian
  • 32,440
  • 24
  • 112
  • 191