Closing resources created in failed constructor

user_677295 0
Viewed 72

Assume I have a class C that holds resources that need to be closed as member variables.

public class C {
    private ClosableResource1 closableResource1;
    private ClosableResource2 closableResource2;
    .....

    public C(){
        closableResource1 = new ClosableResource1(); 
        closableResource2 = new ClosableResource2(); 
        .....
        // some logic that can fail
    }
    
    close(){
        closableResource1.close()
        closableResource2.close()
        .....
    }
}

If the constructor succeeds I can be sure that close() will be called eventually by some entity manager and all the resources will be freed. But how can I make sure I close the resources when the constructor fails? The failure can happen because I have additional logic in the constructor that can throw exception or I get some RuntimeException outside of my control?

Some things I though of:

  • Wrapping the constructor body with a try-catch block. Then, assuming I have a lot of closable members I'll have to have a big if statement in the catch block checking which resources were already initializing and only close them.
  • Offloading the ClosableResources creation to some init() function. Then I would have to make sure init() succeeded every time I try to use the object.

Is there some elegant solution? Or is this much more implementation specific then that?

oop
4 Answers

You can do something like below:

public class C {
    private List<AutoCloseable> closableResources = new ArrayList();
    private ClosableResource1 closableResource1;
    private ClosableResource2 closableResource2;
    .....

    public C() {
        closableResource1 = new ClosableResource1();
        closableResources.add(closableResource1)
        closableResource2 = new ClosableResource2();
        closableResources.add(closableResource2);
        .....
        try {
            // some logic that can fail
        } catch(Exception e) {
            close();
        }
    }
    
    close(){
        for (AutoCloseable closableResource : closableResources) {
            if (closableResource != null) {
                closableResource.close();
            }
        }
    }
}
user_129442330

Surrounding your code with try-catch and closing all your resources in catch is the correct solution here. Also read about method finalize() (Here is one tutorial). In general, I would recommend one method that cleans up all the resources (like you suggested method close(), I would call it though cleanup()) and call that method in your catch section and in your finalize() method

user_58024170

I asked and answered a very similar question here. It is very important that a constructor either succeeds or fails completely i.e. leaving no resources open. In order to achieve that I would follow each resource creation statement by a try-catch block. The catch block closes the resource and rethrows the exception so it is not lost:

  public C() {
    closableResource1 = new ClosableResource1();
    closableResource2 = new ClosableResource2();
    try {
      // .....
      // some logic that can fail and throw MyCheckedException or some RuntimeException
    } catch (RuntimeException | MyCheckedException e) {
      try {closableResource1.close();} catch (Exception ignore) {}
      try {closableResource1.close();} catch (Exception ignore) {}
      throw e;
    }
  }

If creating a resource can fail you need nested try-catch blocks as demonstrated here.

user_12352170

Here's a wild idea: create a class called something like DefusableCloser (that you can "defuse", like an explosive device being made safe):

class DefusableCloser implements AutoCloseable {
  boolean active = true;
  final AutoCloseable closeable;

  DefusableCloser(AutoCloseable closeable) {
    this.closeable = closeable;
  }

  @Override public void close() throws Exception {
    if (active) closeable.close();
  }
}

Now you can use this in a try-with-resources block:

c1 = new CloseableResource();
try (DefusableCloseable d1 = new DefusableCloseable(c1)) {
  c2 = new CloseableResource();
  try (DefusableCloseable d2 = new DefusableCloseable(c2)) {
    // Do the other stuff which might fail...

    // Finally, deactivate the closeables.
    d1.active = d2.active = false;
  }
}

If execution doesn't reach d1.active = d2.active = false;, the two closeables (or one, if the exception was in creating the second resource) will be closed. If execution does reach that line, they won't be closed and you can use them.

The advantage of doing it like this is that the exceptions will be correctly handled.

Note that the ordering is important: don't be tempted to create the two CloseableResources first, then the two DefusableCloseables: doing that won't handle an exception from creating the second CloseableResource. And don't put the creation of the CloseableResources into the TWR, as that would guarantee their closure.

For closing the resources in your class' close() method, you can also use try-with-resources to ensure that both resources are closed:

try (c1; c2) {}

You don't actually have to declare a new variable in the TWR syntax: you can just effectively say "close the resource for this existing variable afterwards", as shown here.

user_37881760
Related