Reputation: 423
I am using Sonar tool to analyze the coding standard in an existing Application where I met a Sonar Rule: "Close the resources" where Connection's object conn is the culprit.
As we know we should close the Connection object using conn.close();
but in the Application a method to release the connection has been called.
Below is the piece of code where Connection object is closed through a method named releaseConnection()
in finally block.
finally {
try {
OtherClass.releaseConnection(conn); // Line: 50 Here is the call to close the conn
}
catch (SomeException se) {
LOGGER.error(" Exception while releaseConnection in add() method : ",se);
}
}
Closing method:
public static void releaseConnection(Connection conn) throws DBException {
if (conn!=null) {
try {
if (!conn.isReadOnly()){
conn.commit();
}
} catch (SQLException e) {
LOGGER.error("Error while commiting the connection. " + e);
rollback(conn);
throw new SomeException(e,SOMETHING);
} finally {
try {conn.close();} catch (SQLException se){
LOGGER.error("releaseConnection() Error " + se);
}
}
}
}
Here's the list of my concern:
As this existing implementation is doing the correct thing (Correct me if I am wrong) is it really need to change the code as per Sonar suggestion.
If really I need to follow the Sonar's suggestion what should be the best way.
UPDATE:
How can I just ignore/bypass some certain code or rule and apply in my above code. Lets say I want to ignore Line: 50, how can I do that?
I do not want to mess with the above piece of code but I really wanna ignore this and make my issues lesser. Thanks in advance.
Upvotes: 2
Views: 831
Reputation: 499
That's right! Sonar does not check if any method is called from finally block to close the resources. It simply checks the closing of resources.
Upvotes: 2
Reputation: 10833
You are actually encountering a limitation of the symbolic execution engine (which is used by this rule under the hood) : https://jira.sonarsource.com/browse/SONARJAVA-1591
What is happening here is that we approximate the flow of execution in try/catch/finally by having a path of execution skipping the whole try block (to simplify the handling of flow) and that results in the false positive you mentioned because we don't see the call to your method that would prevent the issue of being raise.
Upvotes: 2