C# Is it threadsafe to set dictionary object from one thread, get from other thread

Suppose we have some strange class which contains

public property ReadOnlyDictionary<string, string> Map {get; private set;}
method Update which re-sets Map dictionary when called.

Is it threadsafe to call method Update from one thread and get Map from other thread?

public class Example
{
    public ReadOnlyDictionary<string, string> Map{ get; private set; }

    public void Update(IEnumerable<KeyValuePair<string, string>> items)
    {
        Map = items
            .ToLookup(x => x.Key)
            .ToDictionary(x => x.Key, x => x.First())
            .ToReadOnlyDictionary(); // helper method
    }
}

Is it possible that there will be the moment when we call Map getter and it returns null or something in bad state, because of setter execution is not atomic?

Solution

"Thread safe" is a vague term. Wikipedia defines thread-safe code as:

Thread-safe code only manipulates shared data structures in a manner that ensures that all threads behave properly and fulfill their design specifications without unintended interaction

So we cannot say that certain piece of code is "thread safe" without knowing that design specification, which includes how this code is actually used.

This code is "thread-safe" in a sense that it's not possible for any thread to observe Example object in a corrupted or partial state.

Reference assignment is atomic, so thread that reads Map can only read either one vesrion of it or another, it cannot observe partial or intermediate state of Map. Because both keys and values of Map are immutable (strings) - the whole object is immutable and can safely be used by multiple threads after it was read from Map property.

However, doing something like this:

var example = GetExample();
if (example.Map.ContainsKey("key")) {
    var key = example.Map["key"];
}

Is of course not thread safe, because Map object, as a whole, might have changed between check and read from dictionary, in a way that old version contained "key" key, but new version does not. Doing this on the other hand:

var example = GetExample();
var map = example.Map;
if (map.ContainsKey("key")) {
    var key = map["key"];
}

is fine. Any code that does not rely on example.Map to stay the same between reads should be "safe".

So you have to ensure that given code is "thread safe" in your particular use case.

Note that using ConcurrentDictionary is absolutely useless here. Since your dictionary is readonly - it's already safe to read it from multiple threads.

Update: valid point raised in a comment is that ReadOnlyDictionary by itself does not guarantee that underlying data is not modified, because ReadOnlyDictionary is just a wrapper around regular mutable Dictionary.

However, in this case you create ReadOnlyDictionary in Update method from non-shared instance of dictionary (received from ToDictionary), which is not used for anything else except wrapping it in ReadOnlyDictionary. This is a safe usage, because there is no code which has access to mutable underlying dictionary except ReadOnlyDictionary itself, and read only dictionary prevents its modification. Reading from even regular Dictionary is thread safe as long as there are no writes.