Another bad lock pattern

Published 06 December 03 08:59 PM | ricom 

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.

Filed under:

Comments

# Dejan Jelovic said on December 7, 2003 7:25 AM:
Are you regretting your decission to enable any object to be used as a mutex yet? ;>
# Rico Mariani said on December 7, 2003 12:33 PM:
no comment :)
# Ian Ringrose said on December 8, 2003 3:33 AM:
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;
)
# Rico Mariani said on December 8, 2003 4:51 AM:
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.
# Rico Mariani said on December 8, 2003 4:58 AM:
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.
# Wesner Moise said on January 31, 2004 12:47 PM:
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.
# Rico Mariani said on January 31, 2004 1:15 PM:
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.
# Rico Mariani said on January 31, 2004 1:21 PM:
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.
# Mike Volodarsky's ServerSide said on November 10, 2007 9:43 PM:

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

# Blog By Regan said on July 25, 2008 7:34 PM:

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

# Blog By Regan said on July 25, 2008 9:34 PM:

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

# Regan Schroder said on July 28, 2008 1:56 AM:

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

# lock file in ASP.NET | keyongtech said on January 22, 2009 1:49 AM:

PingBack from http://www.keyongtech.com/549706-lock-file-in-asp-net

New Comments to this post are disabled

Search

This Blog

Syndication

Page view tracker