Guillaume, > For Christopher and Alberty, here is an example of how we connect to the > database.
> I'm quite sure we are using the JDBC pool.
From your DataSource configuration, it looks like you are using the
Oracle driver directly. Does that have built-in pooling? If not, it
looks like you might be creating a new connection every time. Sorry, I'm
totally ignorant of the Oracle driver capabilities.
> Connection myConnection = null;
> CallableStatement oCmd = null;
> ResultSet result = null;
>
> try {
> /* SNIP */
> }
> catch (Exception e) {
> logger.error(e.getMessage(), e);
> throw new DepotGarantieNotFoundException("Erreur SQL", e);
> }
> finally {
> try {
> if (result != null) {
> result.close();
> result = null;
> }
> if (oCmd != null) {
> oCmd.close();
> oCmd = null;
> }
> if (myConnection != null) {
> myConnection.close();
> myConnection = null;
> }
> } catch (SQLException e) {}
> }
Although this is probably okay, it's not air tight. You should change
your finally clause to this:
finally {
if (result != null) try { result.close(); }
catch (SQLException e) { logger.error(":(", e); }
if (oCmd != null) try { oCmd.close(); }
catch (SQLException e) { logger.error(":(", e); }
if (myConnection != null) try { myConnection.close(); }
catch (SQLException e) { logger.error(":(", e); }
}
This will attempt to close each resource separately, so if there is some
kind of exception causing your ResultSet to fail to close, you still
have a chance at closing the statement and connection.
Also, I saw that you were swallowing the SQLException at the end of the
finally block. This is a mistake, as you'll never know when you have a
problem! You should always log exceptions if they occur. You never
know... you might be getting exceptions all the time which would lead
you to your connection leak, but never know it because they are being
ignored.
All of the database code in my projects goes into several service
classes to isolate it from the rest of the code. All of the service
classes extend from a BaseService which has the following method:
protected void close(ResultSet rs, PerparedStatement ps,
Connection conn)
{
if (rs != null) try { rs.close(); }
catch (SQLException e) { logger.error(":(", e); }
if (ps != null) try { ps.close(); }
catch (SQLException e) { logger.error(":(", e); }
if (conn != null) try { conn.close(); }
catch (SQLException e) { logger.error(":(", e); }
}
This helps me by putting all of this code together in one place (in case
I needed to change it for whatever reason), and making the methods that
use JDBC a little cleaner because the finally block becomes:
finally
{
super.close(rs, ps, conn);
}
I'm not sure if this will help you too much, but it's an idea that I
have always used.
On a final note, you should be careful when using pooled connections and
transactions. If you don't handle rollbacks properly, you can (at least
with the Jakarta Commons DBCP) end up committing transactions that
should have been rolled back (yikes!).
You need to make sure that you catch /everything/ that can occur, like this:
catch (SQLException) (obvious)
catch (anything in the throws clause of your method)
catch (java.lang.Error)
catch (java.lang.RuntimeException)
This doesn't have anything to do with connection leaks, really, but I
figured I'd mention it.
-chris
signature.asc
Description: OpenPGP digital signature
