Does this simple cache class need thread synchronization?

Question

Does this simple cache class need thread synchronization ... if I remove the lock _syncLock statement will encounter any problems? I think i can remove the locks as the references should be updated correctly right? ... BUt i'm think whar happens if client code is iterating over the GetMyDataStructure method and it get replaced?

Edit: I've replace the GetMyDataStructure with a TryGetValue style method and removed all locking .... this should be ok right?

    public bool TryGetValue(int id, out MyDataStructure myDataStructure)
    {
        return _cache.TryGetValue(id, out myDataStructure);
    }

Thanks!

public sealed class Cache 
{
    private readonly object _syncLock = new object();
    private IDictionary<int, MyDataStructure> _cache;

    public Cache()
    {
        Refresh();
    }

    public void Refresh()
    {
        lock (_syncLock)
        {
            _cache = DAL.GetMyDataStructure();
        }
    }

    public IDictionary<int, MyDataStructure> **GetMyDataStructure**()
    {
        lock (_syncLock)
        {
            return _cache;
        }
    }
}

Answer 1

The Cache class really only tells us about a single reference. That doesn't by itself need synchronization, since it is atomic. However! It really depends what callers are going to do with the dictionary. If they only read, then it will work - but if any of them are going to add/remove/swap values then it will fall apart.

I would be tempted to expose (rather than a dictionary) an immutable type with a similar indexer - so no caller can ruin the party by changing something.

---

After the edit; the version with just TryGetValue no longer makes it possible for callers to break it, so should be fine.

Answer 2

Since assignment to reference types is atomic (see section 5.5 of the C# standard), you don't need a lock in Refresh.

As for locking in GetMyDataStructure, you might actually need that -- for reasons covered here. Though I'm not particularly sure.