Threading Tips: Never Lock a Value Type. Never Lock "This".

0 comments suggest edit

UPDATE: As a commenter pointed out, the original code example did not properly demonstrate the problem with locking on the this keyword within a normal method. I have corrected this example and wrote a better example that demonstrates that this problem still exists even in a “normal” method.

Take a look at this code:

private bool isDisposed = false;
 
//... code...
~MyClass()
{
    lock(isDisposed)
    {
        if(!isDisposed)
        {
            //Do Stuff...
        }
    }
}

Hopefully you can see the problem here right away. The lock statement takes an object instance as a parameter. So what happens to the boolean isDisposed within the lock statement? That’s right! It gets boxed, meaning a new object instance is allocated and passed to the lock statement. Thus every time you lock on a value type, you’re locking on a new object.

Ok, so let’s try to fix this up a bit.

private bool isDisposed = false;
 
//... code...
~MyClass()
{
    lock(this)
    {
        if(!isDisposed)
        {
            //Do Stuff...
        }
    }
}

So is there anything wrong with this? You’ve probably seen the Microsoft examples locking on this. Well never give full trust to example code (especially as it’s unlikely you’ll add the code to the GAC) ;). Suppose this snippet is from a class MyClass. What do you think will happen with the following code:

MyClass instance = new MyClass();

Monitor.Enter(instance);
instance = null;

GC.Collect();
GC.WaitForPendingFinalizers();

You guessed it! Deadlock.

Every time you lock a .NET object, the runtime associates a SyncBlock structure to that object. Locking works by checking who owns an object’s SyncBlock when attempting to acquire a lock. Thus in the code sample above, the client code and the Dispose() Method are attempting to lock on the same object.

For a more in-depth discussion, I highly recommend Jeffrey Richter’s article Safe Thread Synchronization which is where I first learned about this subtle threading issue.

Likewise you might also take a look at Dr Gui’s Don’t Lock Type Objects post.

Found a typo or error? Suggest an edit! If accepted, your contribution is listed automatically here.

Comments

avatar

7 responses

  1. Avatar for Barry Dorrans
    Barry Dorrans April 11th, 2005

    *hanging head in shame now*



    :)

  2. Avatar for jayson knight
    jayson knight April 12th, 2005

    I remember reading somewhere (some blog) that the CLR folks didn't really want to have a lock keyword, but by the time they realized how evil it was, it was too late.



    Something I've always wanted to test (but haven't gotten around to it, or I forget) is locking a shallow copy of an object...would that work? As it creates a proxy back to the original object, I would think it would...



    good post.

  3. Avatar for Haacked
    Haacked April 12th, 2005

    You might have read that on this blog in a post entitled "The Difficulties of Language Design"



    http://haacked.com/archive/2004/05/27/492.aspx

  4. Avatar for Daniel Moth
    Daniel Moth April 16th, 2005

    Similar problems occur when locking on strings (don't!) because of interning...

  5. Avatar for Jack
    Jack August 7th, 2006

    A late comment, since this is still generating bad memes:
    You seem to be implying that multiple locks on the same object from the same thread, will cause deadlock, which is just wrong. Maybe its possible that your example can cause deadlock because it's called from Dispose and that's deadlocking with the GC thread, but I don't see quite how (if it were a destructor, I might believe you). In any case, locking on "this" in an ordinary method is fine.

  6. Avatar for Haacked
    Haacked August 8th, 2006

    @Jack: Whoops, you are right. That should be the finalizer method, not dispose. My code example is not illustrating the point correctly.
    However, the point remains. Locking on "this" in an ordinary method can still cause contention with some external thread locking on the object since they will both lock on the same synch block.
    Read Richter's article on the subject. I'll post a correction to this post.

  7. Avatar for Abhinav
    Abhinav October 3rd, 2008

    I never see Lock(valueType) getting compiled.May I know what am i missing?