Difference between manual locking and Synchronized methods

Ladislav Mrnka picture Ladislav Mrnka · May 26, 2011 · Viewed 34.8k times · Source

Is there any difference between this:

internal class MyClass
{
    private readonly object _syncRoot = new Object();

    public void DoSomething() 
    {
        lock(_syncRoot)
        {
            ...
        }
    }

    public void DoSomethingElse() 
    {
        lock(_syncRoot)
        {
            ...
        }
    }
}

and this:

internal class MyClass
{
    [MethodImpl(MethodImplOptions.Synchronized)]
    public void DoSomething() 
    {
        ...
    }

    [MethodImpl(MethodImplOptions.Synchronized)]
    public void DoSomethingElse() 
    {
        ...
    }
}

The only difference I see is that the first approach locks on some private member whereas the second approach locks on the instance itself (so it should lock everything else in the instance). Is there any general advice which approach to use? I have currently found two classes with similar purpose in our project each written with different approach.

Edit:

Perhaps one more question. Is this:

internal class MyClass
{
    [MethodImpl(MethodImplOptions.Synchronized)]
    public void DoSomething() 
    {
        ...
    }
}

exactly same like this:

internal class MyClass
{
    public void DoSomething() 
    {
        lock(this) 
        {
            ...
        }
    }
}

Answer

Henk Holterman picture Henk Holterman · May 26, 2011

The first method is preferred because you can (and should) make _syncRoot private. This lowers the risk of deadlocking.

The MethodImplOptions.Synchronized is a left-over from an earlier ambitious idea that turned out to be not so good after all.

Regarding the last question: Yes, according to this blog they are functionally equivalent (but not implemented the same way). And all forms of lock(this) are discouraged, again because of deadlock scenarios.