CODEWITHSUNDEEP
java
Will
Will

Reputation: 75635

Ensure objects are closed if an exception is thrown

I am creating JDBC Statements and ResultSets.

Findbugs rightly points out that I don't close these if an exception is thrown.

So now I have:

Statement stmt = null;
ResultSet res = null;
try {
    stmt = ...
    res = stmt.executeQuery(...);
    ...
} finally {
    try {
        if(res != null)
           res.close(); // <-- can throw SQLException
    } finally {
        if(stmt != null)
           stmt.close();
    }
}

(Only I have rather more result sets and prepared statements and so on open... so my nesting of finallys is rather deeper)

There has to a better way to ensure a large number of result sets are closed?

(Aside: in Symbian they never let destructors/close/release/remove -type methods throw any errors. I think this a very good design decision. That all the close methods in JDBC can throw SQLException makes things unnecessarily complicated, in my opinion.)

Upvotes: 6

Views: 186

Answers (5)

Brian Agnew
Brian Agnew

Reputation: 272297

Rather than write this yourself, check out Apache Commons DbUtils.closeQuietly().

This will close combinations of ResultSets, Statements and Connections, handling nulls along the way. It won't handle multiple ResultSets, note, but is useful otherwise.

Upvotes: 3

Duncan Jones
Duncan Jones

Reputation: 69339

If you are using Java 7, then you can take advantage of the fact ResultSet extends AutoCloseable and use a try-with-resources statement.

try (Statement sql = <WHATEVER>;
     ResultsSet res = sql.executeQuery(<WHATEVER>)) {
     // Use results
}

At least then you avoid the finally clauses.

Upvotes: 4

Drew Stephens
Drew Stephens

Reputation: 17827

A blog post by David M. Lloyd from a few years ago covers this well, exploring the options and settling on a pattern of nesting a new try/finally directly above each resource that is created. In our example, something like this:

Statement stmt = null;
ResultsSet res = null;
try {
    stmt = ...
    try {
        res = stmt.executeQuery(...);
        ...
    } finally {
        try {
            res.close();
        } catch (Throwable t) {
            t.printStackTrace();
        }
    }
} finally {
    try {
        stmt.close();
    } catch (Throwable t) {
        t.printStackTrace();
    }
}

If you go this route, it's a good idea to also follow David's advice and create a safe resource closing method that you can use throughout your project. This method can then be called in place of the try/catch blocks within the finally blocks.

// put this anywhere you like in your common code.
public static void safeClose(Closeable c) {
    try {
        c.close();
    } catch (Throwable t) {
        // Resource close failed!  There's only one thing we can do:
        // Log the exception using your favorite logging framework
        t.printStackTrace();
    }
}

Upvotes: 2

T.J. Crowder
T.J. Crowder

Reputation: 1074405

In code where I can rely on Java 7, I'd probably use the try-with-resources as suggested by Duncan Jones.

In my older code, there are two approaches I've used:

The first is a set of helper methods on a static helper class, in this form:

public static final Statement quietClose(Statement s) {
    if (s != null) {
        try {
            s.close();
        }
        catch (Exception e) {
            // Do some useful logging here
        }
    }
    return null;
}

Then in your finally block:

stmt = Helper.quietClose(stmt);

The second approach was to use a LinkedList, add things to it in the order in which I opened them, and then have a helper that looped through in reverse order and basically did the above.

In all cases, I strive to keep the methods short enough that I don't end up with 18 different JDBC objects that I need to close. (I say I "strive"... I don't always succeed.)

Upvotes: 3

John B
John B

Reputation: 32959

To expand on what reporter suggested, maintain a list of elements to close. Create a method to do the close that catches Throwable and returns the Throwable. Iterate the list collecting any Throwables returned and throw the first one returned.

private Throwable close(Closable c){
    try{ 
       c.cloase(); 
       return null;
    } catch (Throwable t){ 
       return t;
    };
}

private void workMethod(){
   try{


   }finally{
         List<Throwable> ts = new ArrayList<Throwable>();
         for(Closable c : closables){
             Throwable T = close(c);
             if (t != null)
                  ts.add(t);
         }

         if (!ts.isEmpty())
             throw ts.get(0);
   }
}

Upvotes: 0

Related Questions