When you use Spring and Ibatis and SQLTemplates, you could have code in your project which looks somewhat like this:
Connection connection = DataSourceUtils.getConnection(getDataSource()); ...<do connection stuff here>... DataSourceUtils.releaseConnection(connection, getDataSource());
Sonar will report that you did not close the connection, while in fact, Spring did that for you. You can not just add a “connection.close()” to the code because the whole point of calling “releaseConnection()” is to have Spring handle all the smart stuff on committing, closing, and returning the connection to the pool if needed.
In our company, not closing the connection is a major blocking violation (and it should be). But in this case, there is no way to make the Jedi wave to Sonar, telling it that “this code will do just fine”. So I added the following trick, albeit a bit dirty:
if (connection.isClosed()){
// This code is only here to keep Sonar from
// warning us that the connection is not
// closed. Please note: Do not just close an
// unclosed connection, Spring should handle
// connection closing and returning to the pool.
connection.close();
}
Use it to your advantage, but use this responsibly. If you see any problems in my solution, or if there is a better way to do this I’d be happy to hear about it.
Edit (2009-12-01):
There is a much better way to do this if you have Spring/Ibatis integration. In stead of DataSourceUtils.getConnection() you can create a new ConnectionCallback object, like so:
JdbcTemplate template = new JdbcTemplate(getDataSource());
Object resultObject = template.execute(new ConnectionCallback() {
public Object doInConnection(Connection conn)
throws SQLException, DataAccessException {
// Do connection stuff here (can return object);
return null;
});
Please note that this code does NOT contain any Connection.close() references. The connection is passed to you, you can use it, and after your method completes, the framework will do whatever is needed to clean everything up. Sonar will not complain because the whole open/close handeling is done outside your method.
Are you trying to close closed connection?
I think there is an exclamation mark missing, and code should be something like this
if (!connection.isClosed()){
connection.close();
}
or am I wrong?
No it is not a Typo. The point is that closing the connection is a task for DataSourceUtils.releaseConnection(). We’re not allowed to close it. That’s why I’m only closing it if it was already closed. The only purpose of doing this is to get Sonar to see the “Connection.close()” statement and think it’s okay.
A collegue had another tip, which I will add to the blogpost later because I have to blog it in it’s simplest form to make it understandable and readable in a blogpost.
Now I get the point! You are going to add this statement before calling DataSourceUtils.releaseConnection() when the connection is still opened so the statement connection.close() will never get executed (because closing the connection is the job for DataSourceUtils.releaseConnection()) and the sonar warning will be gone.
Ofcourse you can do that but I put it after the releaseConnection() to make sure it is the last statement for Sonar’s scanner. But that does not change the behaviour. The connection in this case is Thread bound so it does not matter when you do the if(), it’s not released until your thread ends.
Well, that kind of behavior I could never have guessed from the original post. Now it makes sense to “close a closed connection” an the end of a method. Thanks for explanation.
ik mis ff nog een blog over die paarden uit Friesland, je weet wel die zwarte met die sokken. Hoe heten die nou.
The best approach would be switching of the Sonar check. You are relying on Spring to close the connection, so have some faith and do not pollute your code with not understandable code which have to be read at least twice to understand why you do not use an exclamation mark. Maybe an exclamation mark in the comment would solve that, but I think there is a Sonar rule against that as well.
Of course this is only my opinion.
@Colin Wat dacht je van een Fries?