4

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?

skaffman
  • 398,947
  • 96
  • 818
  • 769
Ann Addicks
  • 915
  • 12
  • 25
  • 3
    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 Jun 19 '09 at 14:23

1 Answers1

9

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.

MarquisDeMizzle
  • 516
  • 9
  • 24
  • Just like to also point out that while closing the connection should clean up the leaked statements, better off to close each one individually. If this code got repeated multiple times in an open connection you'd find yourself running out of resources ( 2 statements at a time =] ) – MarquisDeMizzle Jun 19 '09 at 14:25