Thread Safe Properties in C#

Question

I am trying to create thread safe properties in C# and I want to make sure that I am on the correct path - here is what I have done -

private readonly object AvgBuyPriceLocker = new object(); private double _AvgBuyPrice; private double AvgBuyPrice  {     get     {         lock (AvgBuyPriceLocker)         {             return _AvgBuyPrice;         }     }     set     {         lock (AvgBuyPriceLocker)         {             _AvgBuyPrice = value;         }     } } 

Reading this posting, it would seem as if this isn't the correct way of doing it -

C# thread safety with get/set

however, this article seems to suggest otherwise,

http://www.codeproject.com/KB/cs/Synchronized.aspx

Does anybody have a more definitive answer?

Edit:

The reason that I want to do the Getter/Setter for this property is b/c I actually want it to fire an event when it is set - so the code would actually be like this -

public class PLTracker {      public PLEvents Events;      private readonly object AvgBuyPriceLocker = new object();     private double _AvgBuyPrice;     private double AvgBuyPrice      {         get         {             lock (AvgBuyPriceLocker)             {                 return _AvgBuyPrice;             }         }         set         {             lock (AvgBuyPriceLocker)             {                 Events.AvgBuyPriceUpdate(value);                 _AvgBuyPrice = value;             }         }     } }  public class PLEvents {     public delegate void PLUpdateHandler(double Update);     public event PLUpdateHandler AvgBuyPriceUpdateListener;      public void AvgBuyPriceUpdate(double AvgBuyPrice)     {         lock (this)         {             try             {                 if (AvgBuyPriceUpdateListener!= null)                 {                     AvgBuyPriceUpdateListener(AvgBuyPrice);                 }                 else                 {                     throw new Exception("AvgBuyPriceUpdateListener is null");                 }             }             catch (Exception ex)             {                 Console.WriteLine(ex.Message);             }         }     } } 

I am pretty new to making my code thread safe so please feel free to tell me if I am going about it in the totally wrong way!

Will

Answer 1

The locks, as you have written them are pointless. The thread reading the variable, for example, will:

1. Acquire the lock.
2. Read the value.
3. Release the lock.
4. Use the read value somehow.

There is nothing to stop another thread from modifying the value after step 3. As variable access in .NET is atomic (see caveat below), the lock is not actually achieving much here: merely adding an overhead. Contrast with the unlocked example:

1. Read the value.
2. Use the read value somehow.

Another thread may alter the value between step 1 and 2 and this is no different to the locked example.

If you want to ensure state does not change when you are doing some processing, you must read the value and do the processing using that value within the contex of the lock:

1. Acquire the lock.
2. Read the value.
3. Use the read value somehow.
4. Release the lock.

Having said that, there are cases when you need to lock when accessing a variable. These are usually due to reasons with the underlying processor: a double variable cannot be read or written as a single instruction on a 32 bit machine, for example, so you must lock (or use an alternative strategy) to ensure a corrupt value is not read.