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."

Comments

• Anonymous
  March 22, 2006
  Your T3 isn't going to compile because you can't access a non-static member from a static method.
  
  And your T5 makes too much use of the 'static' keyword as well... DoSomething wouldn't have access to non-static myLock (though DoSomethingElse would have access to static myOtherLock)
  
  But yeah, your point is still valid :-)
• Anonymous
  March 22, 2006
  The comment has been removed
• Anonymous
  March 24, 2006
  The comment has been removed
• Anonymous
  March 24, 2006
  LOL, thanks :)
• Anonymous
  April 26, 2006
  A few days ago I posted a concurrency problem for commentary and I got a very nice set of responses...
• Anonymous
  August 12, 2007
  PingBack from http://andreir.wordpress.com/2007/08/13/lock-uri-in-net-exemple-c-de-la-rico-mariani/