unprotected access to member in property get

Question

I have a property

    public ObservableCollection<string> Name
    {
        get
        {
            return _nameCache;
        }
    }

_nameCache is updated by multiple threads in other class methods. The updates are guarded by a lock. The question is: should I use the same lock around my return statement? Will not using a lock lead to a race condition?

Answer 1

Yes, you should. If you don't add a lock there the newest value of _nameCache might not be returned.

public ObservableCollection<string> Name
{
    get
    {
        lock (_yourLockObject)
        {
            return _nameCache;
        }
    }
}

Answer 2

Hi,

Yes you can use the same lock object. I assmume you have a variable declared such as the following for the lock: private object _lock = new object(); If so you can use the _lock object as in the follwoing block of code.

public ObservableCollection<string> Name 
{ 
    get 
    { 
       lock(_lock)
       {
          return _nameCache; 
       }
    } 
} 

If you did not implement the lock in the situation where multiple threads are apptempting to access _nameCache you might receive a value that was not within the context of the current call to this propery. So yes its mandatory to implement syncronization (the lock) if multiple threads are accessing the same class memeber such as the propery you outlined above.

Enjoy!

Answer 3

It depends on what you mean by updated.

If you mean that the reference is modified, i.e. _nameCache = newvalue;, then as Mark has said, yes you should (with the same lock) and, no, you won't get a race condition.

If, however, you mean that items are added and removed to the instance referenced by _nameCache, then you won't need to lock on return (since the reference itself never changes). However, you will have to be careful how you read the collection after retrieving it - ideally you should then use the same lock before calling any of its methods.

Either that, or you can use the event model to be notified of new items etc if all you need to do is to track changes - since the events will be raised on the thread that currently has the lock to the collection.

If this is not suitable (because you're always getting elements via an indexer or whatever), then you could always return a copy of the ObservableCollection through this property - i.e. return new ObservableCollection<string>(_nameCache);. This will make the return value of the property short-lived, but leaves any caller free to enumerate and index without any fears of state corruption from other threads.