How to use lock wrapper for autocloseable?

Question

I have wrote following wrapepr:

public class AutoCloseableLockWrapper implements AutoCloseable, Lock{
    private final Lock lock;
    public AutoCloseableLockWrapper(Lock l) {
        this.lock = l;
    }
    @Override
    public void lock() {
        this.lock.lock();
    }

    @Override
    public void lockInterruptibly() throws InterruptedException {
        lock.lockInterruptibly();
    }

    @Override
    public boolean tryLock() {
        return lock.tryLock();
    }

    @Override
    public boolean tryLock(long time, TimeUnit unit) throws InterruptedException {
        return lock.tryLock(time,unit);
    }

    @Override
    public void unlock() {
        lock.unlock();
    }

    @Override
    public Condition newCondition() {
        return lock.newCondition();
    }
    @Override
    public void close() {
        this.lock.unlock();
    }
} 

In my code I use it like this:

public class ReadWriteMap implements Map {

    private HashMap map = new HashMap();
    private ReadWriteLock readWriteLock = new ReentrantReadWriteLock();
    private Lock readLock = readWriteLock.readLock();
    private Lock writeLock = readWriteLock.writeLock();

    @Override
    public int size() {
        try (AutoCloseableLockWrapper autoCloseableLockWrapper = new AutoCloseableLockWrapper(readLock)) {
            autoCloseableLockWrapper.lock();
            return map.size();
        }

    }

    @Override
    public boolean isEmpty() {
        try (AutoCloseableLockWrapper autoCloseableLockWrapper = new AutoCloseableLockWrapper(readLock)) {
            autoCloseableLockWrapper.lock();
            return map.isEmpty();
        }
    }

    @Override
    public boolean containsKey(Object key) {
        try (AutoCloseableLockWrapper autoCloseableLockWrapper = new AutoCloseableLockWrapper(readLock)) {
            autoCloseableLockWrapper.lock();
            return map.containsKey(key);
        }
    }
    ...
}

I don't want to create wrapper in each method.

Is there way to combine single wrapper and try with resources ?

Answer 1

You are over-complicating your design. If your AutoCloseableLockWrapper intentionally exposes all operations supported by the underlying Lock, there is no point in making it private and adding delegation methods for each of Lock’s methods. You could simply make the Lock reference public to allow its use, or leave it off entirely, as the code which creates the wrapper already has a reference to the Lock.

All you want to do, is to support a single operation, unlock, which should be viewed as AutoCloseable.

A Java 8 solution may look like

import java.util.concurrent.locks.Lock;

public interface AutoUnlock extends AutoCloseable {
    public static AutoUnlock lock(Lock lock) {
        lock.lock();
        return lock::unlock;
    }

    @Override
    public void close(); // no checked exceptions
}

It can be used like:

Lock lock=…
// …
try(AutoUnlock u=AutoUnlock.lock(lock)) {
    // critical code
}
// …
try(AutoUnlock u=AutoUnlock.lock(lock)) {
    // critical code
}

If you worry about the instance creation (usually this is not an issue), you can re-use AutoCloseables:

AutoUnlock reusable=lock::unlock;
// …
lock.lock();
try(AutoUnlock u=reusable) {
    // critical code
}
// …
lock.lock();
try(AutoUnlock u=reusable) {
    // critical code
}

To me, it looks less clear since the lock(); and try statements are not syntactically coupled and could be separated by accident. But if the lock has a non-local scope, you could solve this by creating a utility method:

final Lock lockInstance; // this field name is to prevent confusion with the lock() method
final AutoUnlock reusable;

YourConstructor(Lock lock) {// you may get the Lock as a parameter
    lockInstance=lock; // or create one here, right in the constructor
    reusable=lockInstance::unlock;
}

AutoUnlock lock() {
    lockInstance.lock();
    return reusable;
}
void doSomething() {
    // …
    try(AutoUnlock u=lock()) {
        // critical code
    }
    // …
    try(AutoUnlock u=lock()) {
        // critical code
    }
}

I think, it’s not too hard to back-port this logic into Java 7 code, if needed.

Answer 2

You can use a factory method that returns a singleton. Nothing is forcing you to use a constructor.

BTW you should not call lock inside the try-block. That should have already happened in the "acquire the resource" phase (within the constructor in your current design, inside the factory method in my proposal).

I see that the above note is already posted on the Q&A page where you contributed your wrapper. The page already has very good content; I advise to study it well.

Answer 3

I'd prefer just creating a new lock (not a wrapper around a lock):

public class AutoReentrantLock implements AutoCloseable {
  private final ReentrantLock lock = new ReentrantLock();

  public AutoReentrantLock lock() {
    lock.lock();

    return this;
  }

  public void earlyUnlock() {
    lock.unlock();
  }

  @Override
  public void close() {
    if(lock.isHeldByCurrentThread()) {
      lock.unlock();
    }
  }
}

Use like this:

private AutoReentrantLock consistencyLock = new AutoReentrantLock();

try(AutoReentrantLock lock = consistencyLock.lock()) {
  // other code
}

Or a more complicated use case, where you unlock halfway:

private AutoReentrantLock consistencyLock = new AutoReentrantLock();

try(AutoReentrantLock lock = consistencyLock.lock()) {
  // Place code here that gathers information (while under lock)        
  // but may exit early or throw exceptions

  lock.earlyUnlock();

  // ... followed by code that is slow that acts upon above gathered information.
}