0

I've been given an assignment to write a max counter class with the following contract:

class MaxCounter
{
private int _value = int.MinValue;

public void Max(int value)
{
    if(value > _value)
    {
        _value = value;
    }
}

public int Value => _value;
}

The assignment asked to implement a thread-safe, non-locking solution using the Interlocked class. I thought about this implementation:

public void Update(int value)
    {
        if (value > Interlocked.Read(ref _value))
        {
            Interlocked.Exchange(ref _value, value);
            Console.WriteLine("Thread {0} updated counter to {1}.", Thread.CurrentThread.ManagedThreadId, value);
        }
    }

But I believe it can be improved with the CompareExchange function. What do you think?

Gilbert Williams
  • 970
  • 2
  • 10
  • 24
  • 1
    Just a nitpick: I think it would be better to match the names of the methods in both of your snippets (your second snippet contains `Update` method instead of `Max`) – TheNightdrivingAvenger Aug 29 '21 at 08:40
  • 1
    Not only "improved". Your solution is not thread-safe, because between the Read and the Exchange another thread could access and update the object. – Klaus Gütter Aug 29 '21 at 08:54

1 Answers1

2

Your current suggested implementation...

... will not work. It's because there's no Interlocked.Read method for Int32.

Even if there existed such a method for Int32, the code would still not work as you expect. You read and update the value not as an atomic operation. After you read the value with Interlocked.Read and right before you do an Interlocking.Exchange, another thread may be scheduled to run while the current thread may be "postponed", so the second thread will update the counter to some other value. When processor time is given again to the first thread, it will update the counter once more with its own value even if it's lower than _value, because the check is already passed.

Proposed solution

Some quick research lead me to this answer. Built-in Interlocked class does not provide the functionality you require. So, you may use the proposed solution from the answer I linked, or try to implement your own solution with memory barriers, for example (since Int32 reads and writes are atomic by design).

TheNightdrivingAvenger
  • 222
  • 2
  • 13