C# Is locking within getters and setters necessary?

Question

Is it necessary to lock getter and setters in general when multiple threads will access a property/field through get/set-functions.

For instance:

You have a timer which regular gets the value of an object. And a process which regular updates this value.

public class Task : IProgressProvider
{
    ...
    public Progress GetProgress()
    {
        // should i lock here?
        return _progress;
    }

    public SetProgress(Progress progress)
    {
        // and lock here?
        _progress = progress;
    }

    public void Execute()
    {
        // ...
        SetProgress(new Progress(10));
        // ...
        SetProgress(new Progress(50));
        // ...
        SetProgress(new Progress(100));
    }
}

public class ProgressReporter
{
    public ProgressReporter()
    {
        _timer = new Timer();
        _timer.Elapsed += Elapsed;
        // ...
    }

    // ...

    public void Elapsed(...)
    {
        var progress = _task.GetProgress();
        // ...
    }
}

The question again:

• Should i lock in the GetProgress and SetProgress function.
• If yes, why?
• Is volatile necessary in this case (assuming one or multiple processor cores)
• To keep it "simple" lets assume the properties/fields from Progress are readonly. EDIT: I know that += and so on are not atomic operations and would require proper handling (f.e. locking)

I would think that the setting and reading from the variable _progress is an atomic operation. It is not important that the GetProgress will get the very very current value. If not this time it will get it on the next call of GetProgress.

Answer 1

If _progress is a reference type, then reading and writing its value (i.e. changing the reference) is indeed an atomic operation, so you wouldn't need to lock for the specific code in your example. But if you ever changed more than one field in the setter or getter, or if the field wasn't a type with atomic reads/writes (e.g. double) then you'd need to lock.

If you want several threads to observe the same value at any instant, then indeed you will need additional locking. If you don't care if one thread could read one value and another thread another value (because it changed inbetween) then locking doesn't really seem necessary.

I would make it volatile for sure. volatile is for exactly that purpose. It will at least prevent the optimising compiler from caching the value when it shouldn't (in case it would ever do such a thing).

So to summarise: For your required usage, you should make _progress volatile, but you don't need to lock access to it.