Welcome to MSDN Blogs Sign in | Join | Help

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.

Published Saturday, December 06, 2003 8:59 PM by ricom
Filed under:

Comments

# re: Another bad lock pattern

Are you regretting your decission to enable any object to be used as a mutex yet? ;>
Sunday, December 07, 2003 7:25 AM by Dejan Jelovic

# re: Another bad lock pattern

no comment :)
Sunday, December 07, 2003 12:33 PM by Rico Mariani

# re: Another bad lock pattern

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;
)
Monday, December 08, 2003 3:33 AM by Ian Ringrose

# re: Another bad lock pattern

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.
Monday, December 08, 2003 4:51 AM by Rico Mariani

# re: Another bad lock pattern

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.
Monday, December 08, 2003 4:58 AM by Rico Mariani

# re: Another bad lock pattern

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.
Saturday, January 31, 2004 12:47 PM by Wesner Moise

# re: Another bad lock pattern

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.
Saturday, January 31, 2004 1:15 PM by Rico Mariani

# re: Another bad lock pattern

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.
Saturday, January 31, 2004 1:21 PM by Rico Mariani

# Integrated mode roadblock: Request is not available in this context exception in Application_Start

The “Request is not available in this context” exception is one of the most common errors

Saturday, November 10, 2007 9:43 PM by Mike Volodarsky's ServerSide

# System.Web.HttpException: Request is not available in this context - Asp.Net Classifieds Starter Kit Does Not Run in IIS7

As we have found in deploying 2 projects to the new release of IIS in the past month that there were

Friday, July 25, 2008 7:34 PM by Blog By Regan

# IIS7 Integrated mode: Request is not available in this context exception in Application_Start

As we have found in deploying 2 projects to the new release of IIS in the past month that there were

Friday, July 25, 2008 9:34 PM by Blog By Regan

# IIS7 Integrated mode: Request is not available in this context exception in Application_Start

The “Request is not available in this context” exception is one of the more common errors you may receive

Monday, July 28, 2008 1:56 AM by Regan Schroder

# lock file in ASP.NET | keyongtech

Thursday, January 22, 2009 1:49 AM by lock file in ASP.NET | keyongtech
New Comments to this post are disabled
 
Page view tracker