Do I get a memory leak by not closing my JDBC PreparedStatements?

Question

I'm working with java.sql PreparedStatements, and I was wondering about the following:

In Java is Pass-by-Value, Dammit!, the following is given as an example of Java's Pass-By conventions:

public void foo(Dog d) {
    d = new Dog("Fifi"); // creating the "Fifi" dog
}

Dog aDog = new Dog("Max"); // creating the "Max" dog
// at this point, aDog points to the "Max" dog
foo(aDog);
// aDog still points to the "Max" dog 

In my code, this comes up as the following (semi-Java pseudocode):

public void method() {
  PreparedStatement pstmt = null;
  ResultSet rs = null;
  try {
    rs = executeStatement(sql-string, pstmt, conn, vars...);
  } catch (....) { /* error-handling */ }
  /// do stuff with the data
  rs.close();
}

where executeStatement is (something similar to) the following:

ResultSet executeStatement(String sql, PreparedStatement pstmt, Connection conn, Object[] vars...) {
  pstmt = conn.prepareStatement(sql);
  /// set pstmt variables...
  ResultSet rs = pstmt.execute();
  return rs;
}

From what I understand of Java's pass-by conventions, it's useless for me to do anything with pstmt in the main code, as it will still be null even after calling executeStatement. However, because closing a PreparedStatement also closes the ResultSet, I know that the PreparedStatement that is created in executeStatement is not closed when I'm processing the ResultSet.

Does this imply that there is a memory leak here? (My understanding of memory leaks and how they can be diagnosed/fixed is spotty at best). Is there any way I could structure this differently to avoid a leak, but continue having a method that can execute an SQL string and return the ResultSet in an abstract manner?

Solution 1

I do not see a memory leak here. Only issue I find is that you are not closing the resultSet in a finally block. So if an exception is thrown then the rs.close() will not get executed.

As Andrei commented, closing the result set will close the underlying statement as well. I am not sure where you close the connection, but that should also happen in a finally block.

Solution 2

First off, memory leaks in the traditional sense, where you have data allocated that cannot be referenced anymore, do not exist in Java, since only what is not referenced, gets collected. However, in this case you're not looking for a memory leak anyway, but a resource leak (which are more of an issue in Java): The memory of the PreparedStatement will be collected eventually and it's memory freed, as it is not referenced anymore after execution of your method, however, the resources hold by the statement should be released much earlier, and not only once the garbage collector runs.

What you can do is write a class that contains both the Statement and the ResultSet as a member and return this class, like so:

class ResultSetStatementPair {
  ResultSetStatementPair(ResultSet rs, Statement stmt) {
    this.rs = rs; this.stmt = stmt;
  }

  ResultSet rs;
  Statement stmt;
}

ResultSetStatementPair executeStatement(String sql, Connection conn, Object[] vars...) {
  PreparedStatement pstmt = conn.prepareStatement(sql);
  ResultSet rs = pstmt.execute();
  return new ResultSetStatementPair(rs, pstmt);
}

public void method() {
  Statement pstmt = null;
  ResultSet rs = null;
  try {
    ResultSetStatementPair pair = executeStatement(sql-string, pstmt, conn, vars...);
    rs = pair.rs;
    stmt = pair.stmt;
    // do stuff with the data
  } catch (....) { /* error-handling */ }
  finally { 
    if(rs != null) rs.close();
    if(stmt != null) stmt.close();
  }
}

Also observe that I added a finally as well as moved the do stuff into the try block.