C# using lock practice

Question

I have critical section in my application which contains a lot of code: which is better way to locking access in threadMethod:

A) lock all block:

private object locker = new object();

private void threadMethod()
{
   while(true)
   {
      lock(locker)
      {
         // do work - a lot of code here
      }
      Thread.Sleep(2000);
   }
}

B) Use additional locked access member canWork:

 private static object locker = new object();
 private bool canWork;
 private bool CanWork
 { 
     get { lock(locker) { return this.canWork; } }
     set { lock(locker) { this.canWork = value; } } 
 }

  private void threadMethod()
  {
     while(true)
     {
        if(CanWork)
        {
           // do work - a lot of code here
        }
        Thread.Sleep(2000);
     }
  }

and somewhere in code

CanWork = false;

Answer 1

Neither is particularly good.

• The first has the disadvantage that you hold the lock for a long time.
• The second has the disadvantage that the state can change after you check it.

Instead try to pass immutable arguments to your method (for example a copy of the data). You will probably still need to lock for constructing the arguments and for collecting the results but this is hopefully a much shorter period of time.