spotbugs / discuss

SpotBugs mailing list
6 stars 1 forks source link

Feedback on OBL_UNSATISFIED_OBLIGATION #63

Closed bazi closed 5 years ago

bazi commented 5 years ago

I know OBL_UNSATISFIED_OBLIGATION is in EXPERIMENTAL category and so I though some feedback on it would be useful.

We have positive bug reports in v3.1.10 for java.sql.ResultSet. ResultSet implements AutoCloseable but usually it is not a resource of a try-with-resources statement because it is closed whenever its associated Statement is closed or re-executed. From API docs of [ResultSet.html#close()](https://docs.oracle.com/en/java/javase/11/docs/api/java.sql/java/sql/ResultSet.html#close())

Sample code below.

try (Connection conn = dataSource.getConnection();
    PreparedStatement ps = conn.prepareStatement(sql)) {
    ps.setString(1, hash);
    ResultSet rs = ps.executeQuery();
    return rs.next();
} catch (SQLException ex) {
    throw new IOException(ex);
}
KengoTODA commented 5 years ago

From my experience, I feel that we cannot trust the documented behaviour of JDBC driver. At least, I know that Oracle's JDBC driver doesn't close ResultSet when we close Statement related with it. For reference:

So to make SpotBugs practical, I recommend us to keep current implementation.

bazi commented 5 years ago

Hmm, that's interesting. Totally agree if there are such exceptional cases. Now I have to make a habit to explicitly close ResultSet as well.

presidentio commented 4 years ago

I want to share the code example when OBL_UNSATISFIED_OBLIGATION error is a false positive.

try {
   PreparedStatement statement = connection.prepareStatement(sql);
   return new JdbcStatementHolder(connection, statement);
} catch (SQLException e) {
   JdbcCommons.closeConnection(connection);
   throw new RuntimeException(e);
}

In this case statement will be closed later by JdbcStatementHolder.

I propose to do not treat the case when the statement is passed to some other Object/method as an error.