Possible resource leak when reusing PreparedStatem

问题:

Suppose you have the following code:

    Connection connection = null;
    PreparedStatement ps = null;

    try {
        Connection = connectionFactory.getConnection();

        ps = statement.prepareStamement(someQuery);
        // execute and read and stuff

        // now you want to use the ps again, since you don't want ps1, ps2, ps3, etc.
        ps = statement.prepareStatement(someOtherQuery); // DOES THIS FORM A POTENTIAL LEAK?
    } catch (a lot of exceptions) {
        // process exceptions
    } finally {

        // close the resources (using util class with null-checks and everything)
        SomeUtilClass.close(ps);
        SomeUtilClass.close(connection);
    }

Is reusing the ps variable a potential leak?

And if so, I would hate to declare multiple of such prepared statements (ps1, ps2, ps3, etc). How should I refactor this?

Thoughts anyone?

EDIT

Receiving several answers stating that this doesn't matter. I would like to point out that I'm experiencing open cursors that stay open a little bit too long, and was wondering if this pattern might have something to do with it.

My thoughts were:

The first statement gets dereferenced and GC'ed, so how does the first PreparedStatement get closed (Database-wise) in this example?

回答1:

This would not necessarily create a leak in a classic, "permanent", sense. The garbage collector will get to the first instance of the prepared statement eventually, and call its finalizer.

However, this is not a good practice to let the garbage collector deal with freeing potentially critical resources, such as DB handles: you should call the close method before reusing the prepared statement variable, or not reuse the variable at all.

try {
    Connection = connectionFactory.getConnection();

    ps = statement.prepareStamement(someQuery);
    // execute and read and stuff

    // now you want to use the ps again, since you don't want ps1, ps2, ps3, etc.
    // v v v v v v v v v v v
    SomeUtilClass.close(ps);
    // ^ ^ ^ ^ ^ ^ ^ ^ ^ ^ ^
    ps = statement.prepareStatement(someOtherQuery); // DOES THIS FORM A POTENTIAL LEAK?
} catch (a lot of exceptions) {
    // process exceptions
} finally {

    // close the resources (using util class with null-checks and everything)
    SomeUtilClass.close(ps);
    SomeUtilClass.close(connection);
}

回答2:

In general yes, this could form a memory leak (of unmanaged resources). Without wishing to state the obvious, if a resource should be closed, then you should close it. In your example you are losing the reference to the first prepared statements and, as such, cannot close them.

It is likely that the GC and finaliser for the leaked reference will perform the neccessary steps to recalim memory from any unmanaged resources acquired, however, best practice dictates that you should be the one to deterministically perform this step.

In your example, in order to close all prepared statements, you could use an Iterable collection like so:

Deque<PreparedStatement> statements = new ArrayDeque<PreparedStatement>();
try {
  statements.addFirst(statement.prepareStamement(someQuery));
  PreparedStatement statement = statements.getFirst();
  ..
}...
finally {
  // enumerate statements and close them.
}

回答3:

No it isn't. That's because the Java runtime is sensible enough to remove a reference from the previous object referred to by ps, prior to assigning it to something else.

If a reference count drops to zero then the object is a candidate for garbage collection whereupon its finaliser will be called and resources released.

回答4:

Re-using a prepared statement variable OF ITSELF does not create a resource leak. The problem here is that you are not closing the first statement before you re-assign it to the second.

You could wrap each prepared statement in its own try/catch block, with a close in the finally block, so you are sure that it gets closed before you go on to the next.

If the desired logic is to abort later queries if the first one fails, you could have nested try/catch blocks, with the inner block closing the prepared statement and then rethrowing the exception or throwing a new exception. But at that point, frankly I think it would be easier and more clear to declare multiple variables.

In general, there's very little gained by reusing a variable. Are you thinking that you want to save memory? I don't know how much memory a PreparedStatement object takes, but I doubt it's more than a few dozen bytes. Unless you have an array with thousands or millions of them, it's just not worth worrying about. If you have to write extra code to support re-using the variable -- like an extra close statement -- that code may take more memory than the extra variable. Re-using variable often makes the code less clear, because now the reader has to figure out, "Oh, at this point it holds the fourth query" or whatever.

(I used to work with a guy who went to great lengths to re-use variables, and he'd re-use them for totally different things. Like at the beginning of a function a certain string variable might hold the customer number but then half way down he'd use it to hold the zip code. Of course that made any name meaningless, so he called all his string variables s1, s2, s3, and so on; all integers i1, i2, i3, etc. Trying to read his programs was a nightmare.)

回答5:

There is no problem in this, as long as the Connection is properly closed. By the way: In this case there's no need to close the (last) statement explicitly, as closing the Connection will do this (and close all other resources acquired through it) for you anyway.

UPDATE It depends on your definition of "a little bit too long". When you close the Connection, all resources allocated through it (including all the statements, possible ResultSets created through these etc.) will be freed ASAP. Depending on when the GC kicks in, the lost PreparedStatements (including their ResultSets etc.) MAY be closed earlier, but nobody can guarantee this. So if you really need to rely on the Statements being closed immediately when they go out of scope, yes, then you must call the respective close method before creating a new one.