views:

206

answers:

2

We've found a bug in old code where connections aren't being closed. It's an easy fix, but I'm wondering how we go about proving that it's fixed. There is a choice of using a connection pool or not. For the pooling use it would be easy to add monitoring for the pool, but when connections pooling is not used, how do we track those unclosed, orphaned connections? Is it just like any other memory leak?

The bug looks like basically a cut and paste error. We have a few classes that manage the DB connection, so it looks roughly like this:

OurDBConn conn1 = ConnectionManager.getConnection();
try {
  // business logic
} catch () {
  //
} finally {
  ConnectionManager.returnConnection(conn1);
}

/// and then later in the same method
OurDBConn conn2 = ConnectionManager.getConnection();
try {
  // business logic
} catch () {
  //
} finally {
  ConnectionManager.returnConnection(conn1); // NOTE Error: conn1 should be conn2
}

I don't know why the earlier coders didn't just reuse the original connection, but that's what it is

(begin edit/append)

Yes, the connection code is ours as well and so I can use the answers given.

However, I don't think I asked the right question, although the answers below answer the question I asked. I'm not sure what the right stackoverflow thing to do is; ask another question, or edit this one?

One of the question I should have asked is: how would these orphaned, un-closed connections manifest themselves in system performance? Also, since these connection objects exist only within the scope of a certain method, wouldn't the connections be eligible for garbage collection? And then if they are gc'ed, what is the effect of open connections being gc'ed?

(end edit)

+5  A: 

Assuming the connection manager is also your own code, you could store the initialised connections (along with a stacktrace) in a map within the connection manager, and then remove them when they are returned. Thus at any point, the keyset of the map is the set of unreturned connections, and you can look up that value in the map in order to find the guilty bit of code that created them and never released them. (If the connection isn't a suitable map key you can probably use some kind of unique ID or connection number or whatever - the actual value doesn't matter so much as its presence).

Then just add some appropriate way to access this map on demand and you're good. Depending on your environment, adding a shutdown hook that dump the contents of the map to a file, and/or adding a JConsole interface to lookup the set of unclosed connections in running code, could both be good options.

If the connection manager isn't your code, you could still probably achieve the same thing using aspects.

Andrzej Doyle
+1 for the AOP approach
teabot
The connection pool that we use does something very like this; I believe it wraps the connections that it returns, and keeps track of when they are used. If a connection is unused for longer than a certain amount of time that we specified in the configuration, the connection will be terminated automatically and the stack trace which was recorded on connection open is printed to the log.
RMorrisey
A: 

You can implement custom mini-framework or use exisitng one as a thin wrapper on JDBC operations. For example there is a spring-jdbc module (mavenized) that covers all of the boilerplate error-prone code from developer.

You can check its usage examples and see that there is no initialization/cleanup at the client code at all! It uses 'template method' pattern, i.e. you just write essential data processing and don't bother with connections/statements/resultsets creation and closing. So, it becomes not possible to introduce the problem you talked at first.

denis.zhdanov