Does lock() makes sense here?

Question

I have a simple class like :

  public class XXX
  {
    double val1;
    double val2;
    double delta;
    public void SetValues(double v1, double v2)
    {
      val1 = v1;
      val2 = v2;
      delta = val1 - val2;
    }

    public double Val1 { get { return val1; } }
    public double Val2 { get { return val2; } }
    public double Delta { get { return delta; } }
  }

, one thread for setting values and multiple threads that read the values. So one can use lock() to make all read and writes uninterrupted. But I am aware that synchronization is never achived, there is always a chance that Val1 - Val2 may not be equal to Delta which I do not care so much. My concern is more about obtaining the stable values via getters. However lock() is expensive for this situation because mostly the readers will work.

The next best thing that come to my mind is using Interlocked.Exchange()

    public void SetValues(double v1, double v2)
    {
      Interlocked.Exchange(ref val1, v1);
      Interlocked.Exchange(ref val2, v2);
      Interlocked.Exchange(ref delta, v1 - v2);
    }

    public double Val1 { 
      get 
      {
        double val = 0;
        Interlocked.Exchange(ref val, val1);
        return val; 
      } 
    }

But the code seems pretty dum to me. I don't know.

So does lock() makes sense? Should I use the Interlocked.Exchange() for performance gain? Or what else can I do?

Answer 1

For multiple readers / writers scenarios I would use ReaderWriterLockSlim.

Represents a lock that is used to manage access to a resource, 
allowing multiple threads for reading or exclusive access for writing.

Use ReaderWriterLockSlim to protect a resource that is read by multiple 
threads and written to by one thread at a time. ReaderWriterLockSlim 
allows multiple threads to be in read mode, allows one thread to be in 
write mode with exclusive ownership of the lock, and allows one thread 
that has read access to be in upgradeable read mode, from which the 
thread can upgrade to write mode without having to relinquish its 
read access to the resource.

Answer 2

You need to lock the whole SetValues method:

private object lockObject = new object();

public void SetValues(double v1, double v2)
{
  lock(lockObject)
  {
    val1 = v1;
    val2 = v2;
    delta = val1 - val2;
  }
}

public double Val1 { get { lock(lockObject) { return val1; } } }
public double Val2 { get { lock(lockObject) { return val2; } } }
public double Delta { get { lock(lockObject) { return delta; } } }

The readers still can get a Val1, Val2 and Delta which do not belong together, because they read it in several steps.

You may put Val1, Val2 and Delta into a value object which van be retrieved in once and which does not change:

public Values Val1
{ 
  get 
  { 
    lock(lockObject) 
    { 
      // create a consistent object which holds a copy of the values
      return new Values(val1, val2, delta); 
    } 
  }
}

struct Values
{
  // ...
  public double Val1 { get /* ... */ }
  public double Val2 { get /* ... */ }
  public double Delta  { get /* ... */ }
}

Answer 3

If you need the getters to return a stable result, and need to maintain only internal synchronization, then I'd recommend you create a class called "Snapshot" (containing Val1,Val2 and Delta) or something similar. In your setter, build a new copy of this class and Exchange it into an instance variable. In your getter, just return the current copy of the snapshot. So long as the callers need a consistent experience, they'd use that single snapshot instance returned from a single getter call.

So you have to abandon having multiple getters - there's no way (without external synchronization) to guarantee that Val1, Val2 and Delta would be consistent otherwise.

---

public class XXX
  {
    public class Snapshot {
      double val1;
      double val2;
      double delta;
      public Snapshot (double val1,double val2)
      {
         this.val1 = val1;
         this.val2 = val2;
         this.delta = val1 - val2;
      }
      public double Val1 { get { return val1; } }
      public double Val2 { get { return val2; } }
      public double Delta { get { return delta; } }
    }
    Snapshot _current;
    public void SetValues(double v1, double v2)
    {
      Snapshot s = new Snapshot(v1,v2);
      /* If there were subsequent steps needed to get the snapshot "ready", you could do them here.
         Otherwise, I think you can do this as a single assignment into _current above */
      _current = s;
    }

    public Snapshot Current { get { return _current; } }

  }