unlocking a lock.tryLock with timeout

Question

I have this code

Lock lock = new ReentrantLock();

void someMethod() {
    try {
        if (lock.tryLock(120, TimeUnit.SECONDS)) {
            // do stuff
        } else {
            // timed out 
            throw new RuntimeException("timeout");
        }
    } finally {
        lock.unlock();
    }
}

This works fine except when when it times out. Since the thread that timesout does not own the lock, IllegalMonitorStateException is thrown. There is no isHeldByCurrentThread in Lock interface. If I dont want to cast to ReentrantLock, I have to use something ugly

...
} finally {
    if (lock.tryLock()) {
        lock.unlock();
        lock.unlock(); // twice because tryLock is called twice
    }
}

or

...
} finally {
    if ( !timeout ) {
        lock.unlock();
    }
}

Any better options ? Thanks

Andy Turner · Accepted Answer · 2017-07-18T15:51:22.577

Only unlock if you acquired the lock:

if (lock.tryLock(120, TimeUnit.SECONDS)) {
  try {
    // Do stuff.
  } finally {
    lock.unlock();
  }
}

Note that:

There is an example like this in the Javadoc;
This idiom doesn't have anything to do with using the timeout: you would not want to unlock the lock of a call to lock() were interrupted. You should always acquire the lock outside, but immediately before, the try.

Sabir Khan · Answer 2 · 2017-07-19T08:08:42.357

try-with-resources is another option ( in addition to Andy's answer ) but Lock is not AutoCloseable .

So you can write a wrappper as explained here in another SO question and here then you can use try-with-resource with this wrapper.

Probably, its not worth the effort if you are not planning to use similar construct at multiple places in your application.

EDIT : Elaborating answer to address Sotirios Delimanolis's concerns .

In my opinion , OP wants to throw a RuntimeException if lock can't be acquired and is confused about closing of lock in finally block since it might throw IllegalMonitorStateException if lock is not already held by thread.

Using Lock gives you flexibility to unlock at any place that programmer wants ( and not necessarily at the end of try block or at the end of method ) & that flexibility is missing in synchronized keyword but as per code snippet of OP, it seems he will unlock just after finishing immediate try block so AutoCloseable makes sense.

Below code addresses both concerns ,

Wrapper class

import java.util.concurrent.TimeUnit;
import java.util.concurrent.locks.ReentrantLock;

public class CloseableReentrantLock extends ReentrantLock implements
        AutoCloseable {

    private static final long serialVersionUID = 1L;


    public CloseableReentrantLock timedLock() throws InterruptedException{
        if(this.tryLock(120, TimeUnit.SECONDS))
            return this;
        else
            throw new RuntimeException("timeout");
    }

    @Override
    public void close() throws Exception {
        this.unlock();

    }

}

Client

try(CloseableReentrantLock lock = new CloseableReentrantLock().timedLock()) { //do stuff here }

Lock Acquired Scenario : Nothing special happens , close method is called after try-with-resource block and lock is unlocked. This is more or less the way synchronized block works but you can't get a waiting time option with synchronized but the finally code cluttering & chances that programmer will miss to code unlock & finally are not there with syncronized and so is the case with Closeable wrapper.

Lock can't be Acquired : In this scenario, since resource is not successfully acquired with in try-with-resource and try-with-resource itself throws Exception, close will not be called on this resource and so IllegalMonitorStateException will not be there. Refer this question and accepted answer to understand more for exceptions from within try-with-resource itself.

This sample code illustrates it further,

public class TryResource implements AutoCloseable{


    public TryResource getResource(boolean isException) throws Exception{
        if ( isException) throw new Exception("Exception from closeable getResource method");
        else return this;
    }

    public void doSomething() throws Exception {
        System.out.println("In doSomething method");
    }

    @Override
    public void close() throws Exception {
        System.out.println("In close method");
        throw new Exception("Exception from closeable close method");
    }

}

and client,

public class TryResourceClient {

    public static void main(String[] args) throws Exception {
        try (TryResource resource = new TryResource().getResource(true)){
            resource.doSomething();
        }
    }

}

How is it an option if it's not `AutoCloseable`? Even with the wrapper, how would it deal with failure to lock? Why bring it up if it's not worth the effort? — Sotirios Delimanolis, Jul 18 '17 at 14:31
I have updated answer to illustrate more about what I meant & what I understood about OP's requirements. On the worth the effort part, I am not sure if he is going to need that syntax one time or thousand times in his application so if you need it thousand times, its better not to clutter your code with finally blocks and let unlocking be done automatically at single place. — Sabir Khan, Jul 19 '17 at 06:10
i have this construct in one place only so will stick with nested try block. but i agree this is much cleaner. thanks for taking time to elaborate — mzzzzb, Jul 20 '17 at 12:57

unlocking a lock.tryLock with timeout

2 Answers2