Most efficent way to use locks for modifying two related fields based on previous values?

StackOverflow https://stackoverflow.com/questions/9961442

  •  28-05-2021
  •  | 
  •  

Frage

I know their are faster\better ways using Interlocked.CompareExchange but I'm just looking for the best way if you are limited to locks.

Joe Albahari says the following:

It’s also possible to safely assign a new ProgressStatus object based on its preceding value (e.g., it’s possible to “increment” the PercentComplete value) — without locking over more than one line of code.

Edit: Updated code based on @drch's answer. Waiting to see if there is a way to do this with a smaller\more efficient lock. This lock has the read, write, and new constructor all in the lock. Was hoping for a smaller\more efficient lock. Thanks.

class ProgressStatus
{
    public readonly int PercentComplete;
    public readonly string StatusMessage;
    public ProgressStatus(int percentComplete, string statusMessage)
    {
        PercentComplete = percentComplete;
        StatusMessage = statusMessage;
    }
}
class Test
{
    readonly object _statusLocker = new object();
    ProgressStatus _status;
    void IncreaseProgress()
    {
        lock (_statusLocker)
          _status = new ProgressStatus(_status.PercentComplete + 10, _status.StatusMessage);
    }
}
War es hilfreich?

Lösung

In the article you mentioned, that only requiring locking on one line was referencing the advanced technique later in Part 5.

Regarding the code you have above, it is possible to get a race condition between your two locks. Two threads could read the same value, and then write the same value when the desired result would be to have them both increment.

Lock over the entire read and write:

lock (_statusLocker) {
    statusCopy = _status;
    var newStatus = new ProgressStatus(statusCopy.PercentComplete + 10, statusCopy.StatusMessage);
    _status = newStatus;
}

Or simply:

lock (_statusLocker) {
   _status = new ProgressStatus(_status.PercentComplete + 10, _status.StatusMessage);
}

Andere Tipps

IMO, using the code above you can do without any locks, since you are using an immutable class and working only with copies of the data. Reading and writing references are guaranteed to be atomic by the C# spec.

Edit: However thread synchronization is a b***, so I would like to hear some more opinions...

Edit: Also as I pointed out in the comments, there is a flaw in the code if more than one thread calling the method. Updates can get lost in this case.

Lizenziert unter: CC-BY-SA mit Zuschreibung
Nicht verbunden mit StackOverflow
scroll top