Nan Wang
Nan Wang

Reputation: 65

How to make sure resource is released, with nested object hierarchy?

I have code (simplified) like this:

class A {

    B b = new B();

    void close() {
        b.close();
    }
}

class B {

    Closeable mustBeClosed = new Closeable() {
        {
            System.out.println("create");
        }
        @Override
        public void close() {
            System.out.println("close");
        }
    };

    int n = 0 / 0;

    void close() {
        mustBeClosed.close();
    }
}

//code
try (A a = new A()) {
    //do something
}

How to guarantee mustBeClosed is released?

This likely happens when the object hierarchy is complex. Override finalize for B might not be a perfect solution.

Any best practice or principle against this issue?

A revised version looks like:

class B {
    Closeable mustBeClosed;
    B() {
        try {

            mustBeClosed = ...

            //other initialization which might raise exceptions

        } catch (throwable t) {
            close();
            throw t;
        }
    }

    void close() {
        if (mustBeClosed != null) {
            try {
                mustBeClosed.close();
            } catch (Throwable t) {
            }
        }
        //all other resources that should be closed
    }
}

However this takes too much code and is far from elegant. What's more, it seems that all classes in the ownership hierarchy should follow the same style, which results lots of code.

Any advice?

Upvotes: 3

Views: 109

Answers (2)

Andreas
Andreas

Reputation: 159135

Your problem is that try-with-resources won't (actually can't) call close() if the constructor throws an exception.

Any object construction that allocates a resource, and has the potential to fail during construction after the resource is allocated, must release that resource before the exception is cascaded up the call stack.

Two ways to fix that:

1) Make sure the resource allocation is the last action performed. In your case, that means move field n up before field mustBeClosed.

2) Handle the resource construction in the constructor, not in a field initializer, so you can catch any subsequent exception and release the resource again before re-throwing the exception, as your alternate solution shows.
However, you don't have to null-check in the close() method, because mustBeClosed will always be non-null if the object construction succeeds, and close() cannot be called if the object construction fails.

Upvotes: 2

TheLostMind
TheLostMind

Reputation: 36304

Use a wrapper method to close all Closeable instances gracefully.

closeGraceFully(Closeable c) { // call this guy for all instances of closeable
   try{
       c.close();
   } catch(IOException ex) {
      // nothing can be done here, frankly.
  }
}

Then call this wrapper method. don't call close() directly. Don't use finalizers they are evil and will slow down your app.

Upvotes: 0

Related Questions