Rico Mariani's Performance Tidbits

Same Resource, Same Lock or else locking won't give you the protection you need

I sometimes see funny locking patterns because there is a mental assumption that the lock keyword (via Monitor features of the runtime) automatically guards the contents of the locked region.  This isn't the case.  All that is guaranteed is that only one thread can hold the monitor (i.e. the locked object) at any moment.  This doesn't result in resource protection at all unless you, by policy, always take the same lock when accessing the same resource.

So here are some (vastly simplified) examples that work:

class T1
{
   private static Object myLock = new Object();

   private Object member1 etc. etc.
   private static Object member2 etc. etc.

   public void DoSomething(T1 him)
   {
       lock (myLock)
       {
        ... use this.member1, him.member1, or member2 // safe
       }
   }
}

class T2
{
   private Object myLock = new Object();
   private Object member1 etc. etc.

   public void DoSomething()
   {
       lock (myLock)
       {
        ... use this.member1 but no other member1 from any other object // safe
       }
   }
}

And here are  some (vastly simplified) examples that do NOT work:

class T3
{
   private Object myLock = new Object();

   private Object member1 etc. etc.
   private static Object member2 etc. etc.

   public void DoSomething()
   {
       lock (myLock)
       {
        ... use member2 // this is not safe
       }
   }
}

class T4
{
   private Object myLock = new Object();
   private Object member1 etc. etc.

   public void DoSomething(T4 him)
   {
       lock (myLock)
       {
        ... use him.member1  // not safe
       }
   }
}

class T5 // the most nefarious of them all
{
   private Object myLock = new Object();
   private static Object myOtherLock = new Object();

   private Object member1 etc. etc.

   public void DoSomething(T1 him)
   {
       lock (myLock)
       {
        ... use this.member1 // not safe due to DoSomethingElse
       }
   }

   public void DoSomethingElse()
   {
       lock (myOtherLock)
       {
        ... use this.member1 // not safe due to DoSomething
       }
   }
}

Of course locking choices can have profound performance ramifications but that's not really the point here.  The thing to remember is that locking alone generally does not provide protection -- you get protection by applying a consistent locking policy to a resource or set of resources.

Sometimes, in the name of performance, complex locking schemes get invented but then race conditions are introduced so correctness goes out the window.  It's always important to remember that, as I'm fond of saying, "It's easy to make it fast if it doesn't have to work."