Another bad lock pattern

A while ago I asked Dr. GUI to post an MSDN article about the perils of using lock(typeof(Foo)), you can still find that article here.

But recently I've started seeing another pattern that's just as bad:

class MyClass
{
    private static String myLock = “MyLock“;

    public void Foo() 
   {
     lock(myLock) { ... }
   }
}

This is bad because string literals are normally interned, meaning that there is one instance of any given string literal for the entire program.  The exact same object represents the literal in all running appdomains, on all threads.  So if someone else comes along and locks a literal named “MyLock” his literal will interfere with yours.

The thing I recommend is:

private static Object myLock = new Object();

That's the safest.

Comments

• Anonymous
  December 06, 2003
  Are you regretting your decission to enable any object to be used as a mutex yet? ;>

• Anonymous
  December 07, 2003
  no comment :)

• Anonymous
  December 07, 2003
  When I implement a singleton in C#, I often use typeof(class), is this a real problem. I do not see how it can give a dead lock. After all one else should be using my class in a lock statement. I agree with using a separate lock object if the locking is any more complex then this.
  
  class A
  (
  private A()
  {
  // some complex code...
  }
  
  public static A Get()
  {
  if (msOneAnyOnly == null)
  {
  lock(typeof(A))
  {
  if (msOneAnyOnly == null)
  {
  msOneAnyOnly = new A();
  }
  }
  }
  
  return msOneAnyOnly;
  }
  
  private A msOneAnyOnly;
  )

• Anonymous
  December 07, 2003
  There are three problems with lock(typeof(A)):
  
  First: type objects are agile, so the same type object is used in all appdomains. That means if your code is running in two different appdomains they will share the same lock which is probably a bad idea.
  
  Second: type objects are public, so anyone anywhere can do lock(typeof(A)) which could totally mess your algorithm up.
  
  Third: typeof() isn't free, you'll have to drag in a bunch of reflection code.
  
  So, avoiding lock(typeof(X)) results in faster, simpler, and more correct code. In contrast I can't think of any benefits to using lock(typeof(X)) under any circumstances.

• Anonymous
  December 07, 2003
  Clarification: In your particular example the first problem doesn't matter much. Really the third is the big issue. It's much cheaper to have a static variable that holds the lock explicitly.

• Anonymous
  January 31, 2004
  In an NGEN app, doesn't using a static initializer introduce new additional checks before every function call?
  
  Thus, in fixing the lock, we slowed the class down.

• Anonymous
  January 31, 2004
  I think typically not. If you put the test in the constructor for instance, then you mostly don't have to worry about instance methods being called having not initialized the statics because if you have an instance then you necessarily have run the constructor.
  
  There are complicated rules for this but it ends up not costing too much. Also this is only an issue for domain nuetral code.
  
  In any case, the cost of typeof() is greater than just a single test... even if there was an extra test in every method that used the lock, it would still be faster.

• Anonymous
  January 31, 2004
  Actually, I think going into the exact details of this process would make an interesting blog/article. First I'll see if someone has already done a good job on this topic.

• Anonymous
  November 10, 2007
  The “Request is not available in this context” exception is one of the most common errors

• Anonymous
  July 25, 2008
  As we have found in deploying 2 projects to the new release of IIS in the past month that there were

• Anonymous
  July 25, 2008
  As we have found in deploying 2 projects to the new release of IIS in the past month that there were

• Anonymous
  July 27, 2008
  The “Request is not available in this context” exception is one of the more common errors you may receive

• Anonymous
  January 21, 2009
  PingBack from http://www.keyongtech.com/549706-lock-file-in-asp-net