4
votes

I ran findbugs on our code base and it pointed out there are two more Statements that still need to be closed. In this section of the code we run:

preparedStatement = connection.prepareStatement(query);

for 3 different queries, reusing preparedStatement. In the finally block we do close the resource:

finally{
   try{
      if (resultSet != null) 
         resultSet.close();
   } catch (Exception e) {
      exceptionHandler.ignore(e);
   }
   try {
      if (preparedStatement != null) 
         preparedStatement.close();
   } catch(Exception e) {
      exceptionHandler.ignore(e);
   }

Should the statement be closed before the next connection.prepareStatement(query); or is this findbugs being cautious?

1
Whether or not you need the reference is inconsequential. When you call connection.prepareStatement(query) you are creating a preparedStatement in that connection. Those are going to sit there until the connection is closed. When you hit that finally you are only closing the prepared statement pointed to by that variable. Leaving 2 of the 3 until the connection is closed. In production you will continue to leak connections and eventually run out if you don't removeAbandoned(tomcat) or the like. - Zach
Thanks for the explanation! Added the changes :) - Ann Addicks

1 Answers

9
votes

Yes, the statement must be closed before you perform the next connection.prepareStatement. Otherwise, you're losing your reference to the un-closed previous one (aka leaking statements). Wrap a try {} finally {} around each statement use, closing it in the finally.