Make property thread safe in c# with a semaphore or lock?

Question

I'm new about reactive, but the idea turned my curiosity on, so I've done a little kinda reactive variable, but I dunno if this is best practice, and then I would turn this thread-safe.

Here's the class:

 public class RxVar<T>:IObservable<T>
 {

     T _value;
     public T Value
     {

         get{ 
             return _value;
         }
         set{
             if (!_value.Equals(value))
             {
                 onChange(_value);
                 _value = value;
             }
         }

     }


     event onValChange onChange = delegate { };
     delegate void onValChange(T val);

     IObservable<T> _observable;
     public RxVar() {

         _observable = Observable.FromEvent<onValChange,T>(ev => onChange += ev, ev => onChange -= ev);
     }

     public IDisposable Subscribe(IObserver<T> observer)
     {
         return _observable.Subscribe(p=>observer.OnNext(p));
     }
 }

Semaphore example

         _semaphoreSlim.WaitOne();
         if (!_value.Equals(value))
         {
             onChange(_value);
             _value = value;
         }
         _semaphoreSlim.release();

Ok, I'd like to make this thread safe, and I'm scared about deadlocks. So, it's better using lock or semaphore, or due to reactive nature is it not needed?

Thanks. :)

Answer 1

I'd recommend a lock instead of a semaphore. A semaphore is simply the wrong tool for this case.

Consider that exposing the actual value with "return _value" means that even after the getter has returned, client code will be able to modify the data in the object. So the question is how will you avoid race conditions after you've exposed the reference?

The answer is don't expose the reference. Give out a DEEP COPY of the object/data, not the original object itself.

About deadlocks: you shouldn't be scared at all if you know what you are doing. If you don't, there is NO WAY you'll be able to tell that there never will be deadlocks, because on each run conditions may vary and even after many successful tests you might cause a deadlock and debugging it will be a difficult undertaking.

Deadlocks can only be avoided by studying them beforehand, not by any means of "programming without knowing".

The small snippet with lock:

Class instance field (you need this if _value can be null, otherwise you can lock on _value itself):

private object _valueLock = new Int(0);

In your methods:

lock(_valueLock)
{
    if (_value != value) // this or deep equality
    {
        _value = value;
        onChange(this, _value); // for complex Observer-Observable scenarios it's better specifying the observed object
    }
}