How to properly scope a lock

Question

Let's say I have a ReentrantLock,

ReentrantLock foo = new ReentrantLock();

and a method bar that uses the lock,

public void bar()
{
    foo.lock();

    try
    {
        methodOne();
    }
    finally
    {
        foo.unlock();
    }
}

and methodOne calls a method methodTwo that also uses the lock,

public void methodTwo()
{
    foo.lock();

    try
    {
        // do something
    }
    finally
    {
        foo.unlock();
    }
}

is it better to release the lock in bar before I call methodOne? In general, is it good practice to release the lock before calling another method?

Answer 1

No, you should not do that. If you do it, any concurrent thread could acquire the lock in the middle of the bar() method, which is precisely what you want to avoid by using a lock.

It's called ReentrantLock precisely because it allows acquiring the lock multiple times in the same thread. A counter is incremented each time lock() is called, and decremented each time unlock() is called, so you just need to make sure unlock() is always called in a finally block.

Note that the code would be less fragile by simply using synchronized blocks, because it's impossible to forget closing a synchronized block.

Answer 2

Release the lock when the critical operations the lock "protects" are done. No sooner.