How to correctly implement multi-threading in C#

Once you fully understand these your ready write what will prove to be a lot of broken multi-threaded code. Go wild, have fun, and enjoy it… just don’t put it in a product.  Failure is the ultimate, and often only, teacher.  Such is the case with multi-threaded code.  If you want to expedite your failures, use the CHESS program by Microsoft.Some things to consider:

While writing anything thread-safe, ask yourself the following questions:

1. What can happen if two threads are executing each statement in step with each-other?
2. Do I really need this lock()?
   1. You need a lock if you are going read AND write to memory that other threads will also access.
   2. Consider using Interlocked instead.
3. What’s the least amount of code that MUST be in the lock()?
4. Can I make this object/state immutable and avoid the locking?
5. What’s the harm if I don’t lock it?
6. Is there any way for a caller to have code execute inside the lock?
   1. Do not call delegates inside a lock unless you created it.
   2. Do not call methods/properties inside a lock on a class you didn’t create or don’t control.
   3. Never invoke to a Control from within a lock.
7. Review all members accessed and changed inside a lock for one of these:
   1. Was the variable declared ‘volatile’?
   2. Did I use Thread.VolatileRead/Write?
   3. Did I use Thread.MemoryBarrier?
8. Are you using System.Threading.Thread.MemoryBarrier correctly?
9. If multiple locks are involved are they always obtained in the same order?
10. Don’t use a private member (or expose if you must) to synchronize as this prevents ordering locks.

 
Things you may not need to make thread-safe:

1. Quite commonly you find people locking on initialization of a property when performing a lazy-load:

		private volatile object _data;
		public object Data
		{
			get
			{
				if (_data != null) return _data;
				lock (this)
				{
					if (_data == null)
						_data = new object();
				}
				return _data;
			}
		}

This is usually not necessary since the worst that happens is the data is loaded twice.  It depends greatly on what that object is and can do, but usually it’s safe to load two instances.  Think about it without concern for performance (the likely-hood is too small to be a performance impact). If you can load/create the data twice without causing issue then just do this instead:

		public object Data
		{
			get { return _data ?? _data = new object(); }
		}

2. Consider pre-fetching instead of lazy loading with shared state.  This can then be performed in the constructor which is only on a single thread.

3. Immutable objects (objects that do not change state) do not normally need locks to access.

4. Fire-and-forget objects work best, start a task and don’t wait for it.  You defeat the purpose of multi-threading with synchronization.

And finally, yes you should test it but don’t depend upon it. Get the whole team together if needed to do a line-by-line walk of the code. I’m sure I’ve missed a lot of things here, but this should be a good start.