Best design pattern for managing asymmetrical resource use

Question

I wanted to canvas some opinions on the best design pattern for working with managed resources, where two distinct resources are involved but you need to release them in the opposite order to that which they were acquired.

First, let me set the scene. We are working with two types of objects Documents, and Collections of Documents. A Collection of Documents literally contains references to the Documents and some metadata per-document.

Originally we had a symmetrical pattern which flowed like:

1. Lock Collection
2. Do useful stuff with Collection
3. Lock Document
4. Do useful stuff with Collection and Document
5. Unlock Document
6. Unlock Collection

and in code was represented like:

Collection col = null;
try {
    col = getCollection("col1 name", LockMode.WRITE_LOCK);

    // Here we do any operations that only require the Collection

    Document doc = null;
    try {
        doc = col.getDocument("doc1 name", LockMode.WRITE_LOCK);

        // Here we do some operations on the document (of the Collection)

    } finally {
        if (doc != null) {
            doc.close();
        }
    }

} finally {
    if (col != null) {
        col.close();
    }
}

Now that we have try-with-resources since Java 7, we have improved this so that Java code delineation automatically releases the resources:

try (final Collection col = getCollection("col1 name", LockMode.WRITE_LOCK)) {

    // Here we do any operations that only require the Collection

    try (final Document doc = col.getDocument("doc1 name", LockMode.WRITE_LOCK)) {

        // Here we do some operations on the document (of the Collection)

    }

}

The problem we have is that keeping the Collection locked whilst we perform operations on the document is inefficient, as other threads have to wait, and often the operations on the document don't require modifying the Collection.

So we would like to move to an asymmetrical pattern which allows us to release the Collection as soon as possible. The flow should be like:

1. Lock Collection
2. Do useful stuff with Collection
3. Lock Document
4. Do anything that requires both Collection and Document (rare)
5. Unlock Collection
6. Do useful stuff with Document
7. Unlock Document

I am wondering about the best pattern for implementing this asymmetrical approach in code. This could obviously be done with try/finally etc like so:

Collection col = null;
Document doc = null;
try {
    col = getCollection("col1 name", LockMode.WRITE_LOCK);

    // Here we do any operations that only require the Collection
    try {
        doc = col.getDocument("doc1 name", LockMode.WRITE_LOCK);

        // Here we do any operations that require both the Collection and Document (rare).

    } finally {
        if (col != null) {
        col.close();
    }

    // Here we do some operations on the document (of the Collection)

} finally {
    if (doc != null) {
            doc.close();
        }
    }
}

I can also think of a try-with-resources scheme where we exchange the resource release order but I am wondering if that makes reading the code less understandable. For example:

try (final ManagedRelease<Collection> mcol =
        new ManagedRelease<>(getCollection("col1 name", LockMode.WRITE_LOCK))) {

    // Here we do any operations that only require the Collection

    try (final ManagedRelease<Document> mdoc =
            mcol.withAsymetrical(mcol.resource.getDocument("doc1 name", LockMode.WRITE_LOCK))) {

        // Here we do any operations that require both the Collection and Document (rare).

    }  // NOTE: Collection is released here

    // Here we do some operations on the document (of the Collection)

}  // NOTE: Document is released here

The ManagedRelease class:

private static class ManagedRelease<T extends AutoCloseable> implements AutoCloseable {
    final T resource;
    private Supplier<Optional<Exception>> closer;

    public ManagedRelease(final T resource) {
        this.resource = resource;
        this.closer = asCloserFn(resource);
    }

    private ManagedRelease(final T resource, final Supplier<Optional<Exception>> closer) {
        this.resource = resource;
        this.closer = closer;
    }

    public <U extends AutoCloseable> ManagedRelease<U> withAsymetrical(final U otherResource) {
        // switch the closers of ManagedRelease<T> and ManagedRelease<U>
        final ManagedRelease<U> asymManagedResource = new ManagedRelease<>(otherResource, closer);
        this.closer = asCloserFn(otherResource);
        return asymManagedResource;
    }

    @Override
    public void close() throws Exception {
        final Optional<Exception> maybeEx = closer.get();
        if(maybeEx.isPresent()) {
            throw maybeEx.get();
        }
    }

    private static Supplier<Optional<Exception>> asCloserFn(final AutoCloseable autoCloseable) {
        return () -> {
            try {
                autoCloseable.close();
                return Optional.empty();
            } catch (final Exception e) {
                return Optional.of(e);
            }
        };
    }
}

I would welcome opinions on whether the try-with-resources approach to asymmetrical resource management is a sensible one or not, and also any pointers to other patterns that might be more appropriate.

Answer 1

The first question seems to be under-specified expected behavior. Particularly, if Collection.close throws an Exception, what should happen? Should Document processing be continued? Should part of the processing of the Document done under both locks be rolled back?

If the answer is Collection.close never actually throws any exceptions (or you don't care what happens if it does), IMHO the simplest solution is to make your Collection.close idempotent and then explicitly call it in the middle of the try-with-resources block where it is appropriate. Also it makes a great sense to make "usual" Collection methods raise something like IllegalStateException if called on a closed Collection. Then you second example would become something like this:

try (final Collection col = getCollection("col1 name", LockMode.WRITE_LOCK)) {
    // Here we do any operations that only require the Collection

    try (final Document doc = col.getDocument("doc1 name", LockMode.WRITE_LOCK)) {

        // Here we do any operations that require both the Collection and Document (rare).


        // NOTE: usually Collection is released here
        col.close();
        // optionally make `col` not final and explicitly set it to `null`
        // here so IDE would notify you about any usage after this point

        // Here we do some operations on the document (of the Collection)

    }  
}  

If you can't change Collection.close code, you can change your ReleaseManager to make close idempotent. Optionally you might also rename it to something like ResourceManager. add a getter there and always access resource only via that getter. And the getter will throw IllegalStateException if called after close.

If Collection.close may actually throw some exception and you do care about such scenarios, it is hard to provide a solution without knowing what expected behavior is.

Answer 2

I will give you a common, complete and chaining solution like this:

   public static void sample() {
    Resource resourceA = new Resource("A");
    Resource resourceB = new Resource("B");
    LockVisitor.create(resourceA)
        .lock()// lock A
        .doOnValue(Main::doSomething)// do for A
        .with(resourceB)// join with B
        .lock()// lock A & B (A has been locked)
        .doOnBoth(Main::doSomething)// do for A and B
        .toRight()// only need B (unlock A)
        .doOnValue(Main::doSomething)// do for B
        .close();// unlock B
  }

  private static void doSomething(Resource... rs) {
    System.out.println("do with: " + Arrays.toString(rs));
  }

and sample will output what is you expected:

lock: Resource(A)
do with: [Resource(A)]
lock: Resource(B)
do with: [Resource(A), Resource(B)]
unlock: Resource(A)
do with: [Resource(B)]
unlock: Resource(B)

First, we should define the lockable resource. How to lock and how to unlock.

public interface Lockable extends AutoCloseable {

  void lock() throws Exception;

  void unlock() throws Exception;

  boolean isLocked();

  @Override
  default void close() throws Exception {
    unlock();
  }
}

You can let your class implement this interface for more clear call.

Then we can build our LockVisitor (for reduce the length of this answer, I delete method implemention. You can find the complete code on github.)

import io.reactivex.functions.Consumer;

public class LockVisitor<T extends Lockable> implements AutoCloseable {
  public static <T extends Lockable> LockVisitor<T> create(T lockable) {
    return new LockVisitor<>(lockable);
  }

  T value;
  Exception error;

  public LockVisitor(T value);

  public LockVisitor<T> lock();

  public LockVisitor<T> unlock();

  public LockVisitor<T> doOnValue(Consumer<T> func);

  public LockVisitor<T> doOnError(Consumer<Exception> func);

  public <B extends Lockable> TwoLockVisitor<T, B> with(LockVisitor<B> other);

  public <B extends Lockable> TwoLockVisitor<T, B> with(B other);
}

and our TwoLockVisitor for visit two resources together:

import io.reactivex.functions.BiConsumer;
import io.reactivex.functions.Consumer;

public class TwoLockVisitor<A extends Lockable, B extends Lockable> {
  public static <A extends Lockable, B extends Lockable> TwoLockVisitor<A, B> create(A a, B b) {
    return new TwoLockVisitor<>(LockVisitor.create(a), LockVisitor.create(b));
  }

  LockVisitor<A> left;
  LockVisitor<B> right;

  public TwoLockVisitor(LockVisitor<A> left, LockVisitor<B> right);

  public TwoLockVisitor<A, B> lock();

  public TwoLockVisitor<A, B> unlock();

  public TwoLockVisitor<A, B> doOnLeft(Consumer<A> func);

  public TwoLockVisitor<A, B> doOnRight(Consumer<B> func);

  public TwoLockVisitor<A, B> doOnBoth(BiConsumer<A, B> func);

  public LockVisitor<A> toLeft();

  public LockVisitor<B> toRight();
}

Now, you can use the classes to managing your resource with any order.

Answer 3

Your ManagedRelease scheme definitely does make the code less understandable. The most direct explicit writeup of your intentions using language features goes like this:

try (final Collection col = getCollection("col1 name", LockMode.WRITE_LOCK)) {

    // Here we do any operations that only require the Collection

}
try (final Collection col = getCollection("col1 name", LockMode.WRITE_LOCK;
    final Document doc = col.getDocument("doc1 name", LockMode.WRITE_LOCK)) {

    // Here we do any operations that require both the Collection and Document (rare).

}
try (final Document doc = col.getDocument("doc1 name", LockMode.WRITE_LOCK)) {

    // Here we do some operations on the document (of the Collection)

}

The problem with this is the extra release and re-acquisition of each lock, and also that col is out of scope for the last getDocument call so it won't quite compile as is.

I would suggest resolving this with a different take on the ManagedRelease concept, lifted up one level. The usage pattern I envision for this would work like this:

// The lambdas here are Supplier
try (final ReleaseManager<Collection> colManager = new ReleaseManager<>(() -> getCollection("col1 name", LockMode.WRITE_LOCK);
    final ReleaseManager<Document> docManager = new ReleaseManager<>(() -> colManager.getResource().get().getDocument("doc1 name", LockMode.WRITE_LOCK)) {

    try (final Managed<Collection> colManaged = colManager.getResource()) {

        // Here we do any operations that only require the Collection

    } // Here the resource close does nothing

    try (final Managed<Collection> colManaged = colManager.getResourceForLastUse();
        final Managed<Document> docManaged = docManager.getResource()) {

        // Here we do any operations that require both the Collection and Document (rare).

    } // Here the close of colManaged actually closes it, while docManaged.close() is a no-op

    try (final Managed<Document> docManaged = docManager.getResourceForLastUse()) {

        // Here we do some operations on the document (of the Collection)

    } // Here the document gets closed
} // Here the managers get closed, which would close their resources if needed

This has the same clarity of which resources are used in each block, uses the try-with-resources language feature, releases each resource immediately after its last use, and only acquires each lock once.

For the specification of ReleaseManager:

ReleaseManager here is a generic class that takes a Supplier for a resource, invokes it lazily on the first getResource() call, and memoizes the result for future calls. getResource() returns a wrapper that does nothing when closed, getResourceForLastUse() returns a wrapper that actually does close the resource when the wrapper is closed; I wrote these as being the same class, but you could make them different classes instead, I'm not sure if it really makes anything clearer.

ReleaseManager itself also implements AutoCloseable, and its close() implementation is a failsafe that closes the resource if it has been gotten but not closed. I would consider having it also log a warning in some manner, to draw attention in case a resource's last use is not properly declared to be its last. And for one final consideration, both resource retrieval methods should throw if the resource has already been closed.

I'm leaving the implementation of ReleaseManager as an exercise for you if you like this solution.