Is this example thread safe?

Question

Suppose I have singleton class that acts as a data cache. Multiple threads will read from the cache, and a single thread will periodically refresh it. It looks something like this:

public sealed class DataStore
{
    public static DataStore Instance { get { return _instance; } }
    public Dictionary<Foo, Bar> FooBar { get; private set; }

    static DataStore() { }
    private DataStore() { }

    public void Refresh() {
        FooBar = GetFooBarFromDB();
    }

    private static readonly DataStore _instance = new DataStore();
}

My question is essentially, is it safe to Refresh() while other threads may be accessing FooBar? Do I need to be using locks, or are my getting and setting operations atomic? Do I need to explicitly declare volatile fields to back up my properties?

P.S., If someone can think of a more descriptive title for this question, I would gladly welcome it.

Edit: Fixed my example to correct obviously non-atomic code.

Answer 1

Yes, in cases like that you need explicit synchronization, because another thread could get FooBar and start reading it before you have finished writing.

If you do this, however,

public void Refresh() {
    var tmp = new Dictionary<Foo, Bar>();
    // Fill out FooBar from DB
    FooBar = tmp;
}

then you wouldn't need to add explicit synchronization, because the switch from one reference over to the other reference is atomic.

Of course there is an implicit assumption here that there is no writing outside the Refresh method.

EDIT: You also should switch from an auto-implemented property to a manually implemented property with a backing variable declared with volatile modifier.