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
  • 98,240
  • 88
  • 296
  • 433
Mark 909
  • 1,825
  • 1
  • 18
  • 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
  • 1,026,079
  • 266
  • 2,566
  • 2,900
  • 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
  • 1,023,142
  • 271
  • 3,287
  • 2,928
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
  • 20,076
  • 6
  • 59
  • 90
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
  • 33,605
  • 26
  • 118
  • 198