How to easy make this counter property thread safe?

Question

I have property definition in class where i have only Counters, this must be thread-safe and this isn't because get and set is not in same lock, How to do that?

    private int _DoneCounter;
    public int DoneCounter
    {
        get
        {
            return _DoneCounter;
        }
        set
        {
            lock (sync)
            {
                _DoneCounter = value;
            }
        }
    }

Answer 1

If you're looking to implement the property in such a way that DoneCounter = DoneCounter + 1 is guaranteed not to be subject to race conditions, it can't be done in the property's implementation. That operation is not atomic, it actually three distinct steps:

1. Retrieve the value of DoneCounter.
2. Add 1
3. Store the result in DoneCounter.

You have to guard against the possibility that a context switch could happen in between any of those steps. Locking inside the getter or setter won't help, because that lock's scope exists entirely within one of the steps (either 1 or 3). If you want to make sure all three steps happen together without being interrupted, then your synchronization has to cover all three steps. Which means it has to happen in a context that contains all three of them. That's probably going to end up being code that does not belong to whatever class contains the DoneCounter property.

It is the responsibility of the person using your object to take care of thread safety. In general, no class that has read/write fields or properties can be made "thread-safe" in this manner. However, if you can change the class's interface so that setters aren't necessary, then it is possible to make it more thread-safe. For example, if you know that DoneCounter only increments and decrements, then you could re-implement it like so:

private int _doneCounter;
public int DoneCounter { get { return _doneCounter; } }
public int IncrementDoneCounter() { return Interlocked.Increment(ref _doneCounter); }
public int DecrementDoneCounter() { return Interlocked.Decrement(ref _doneCounter); }

Answer 2

Using the Interlocked class provides for atomic operations, i.e. inherently threadsafe as in this LinqPad example:

void Main()
{
    var counters = new Counters();
    counters.DoneCounter += 34;
    var val = counters.DoneCounter;
    val.Dump(); // 34
}

public class Counters
{
    int doneCounter = 0;
    public int DoneCounter
    {
        get { return Interlocked.CompareExchange(ref doneCounter, 0, 0); }
        set { Interlocked.Exchange(ref doneCounter, value); }
    }
}

Answer 3

What exactly are you trying to do with the counters? Locks don't really do much with integer properties, since reads and writes of integers are atomic with or without locking. The only benefit one can get from locks is the addition of memory barriers; one can achieve the same effect by using Threading.Thread.MemoryBarrier() before and after you read or write a shared variable.

I suspect your real problem is that you are trying to do something like "DoneCounter+=1", which--even with locking--would perform the following sequence of events:

  Acquire lock
  Get _DoneCounter
  Release lock
  Add one to value that was read
  Acquire lock
  Set _DoneCounter to computed value
  Release lock

Not very helpful, since the value might change between the get and set. What would be needed would be a method that would perform the get, computation, and set without any intervening operations. There are three ways this can be accomplished:

1. Acquire and keep a lock during the whole operation
2. Use Threading.Interlocked.Increment to add a value to _Counter
3. Use a Threading.Interlocked.CompareExchange loop to update _Counter

Using any of these approaches, it's possible to compute a new value of _Counter based on the old value, in such a fashion that the value written is guaranteed to be based upon the value _Counter had at the time of the write.

Answer 4

If you're expecting not just that some threads will occasionally write to the counter at the same time, but that lots of threads will keep doing so, then you want to have several counters, at least one cache-line apart from each other, and have different threads write to different counters, summing them when you need the tally.

This keeps most threads out of each others ways, which stops them from flushing each others values out of the cores, and slowing each other up. (You still need interlocked unless you can guarantee each thread will stay separate).

For the vast majority of cases, you just need to make sure the occasional bit of contention doesn't mess up the values, in which case Sean U's answer is better in every way (striped counters like this are slower for uncontested use).

Answer 5

You could declare the _DoneCounter variable as "volatile", to make it thread-safe. See this:

http://msdn.microsoft.com/en-us/library/x13ttww7%28v=vs.71%29.aspx