Java 方法可能无法在检查异常时清理流或资源——FindBugs
声明：本页面是StackOverFlow热门问题的中英对照翻译，遵循CC BY-SA 4.0协议，如果您需要使用它，必须同样遵循CC BY-SA许可，注明原文地址和作者信息，同时你必须将它归于原作者（不是我）：StackOverFlow
原文地址: http://stackoverflow.com/questions/23961553/
Warning: these are provided under cc-by-sa 4.0 license. You are free to use/share it, But you must attribute it to the original authors (not me):
StackOverFlow
Method may fail to clean up stream or resource on checked exception -- FindBugs
提问by Unknown
I am using Spring JDBCTemplate to access data in database and its working fine. But FindBugs is pointing me a Minor issue in my code snippet.
我正在使用 Spring JDBCTemplate 访问数据库中的数据并且它工作正常。但是 FindBugs 在我的代码片段中指出了一个小问题。
CODE:
代码：
public String createUser(final User user) {
try {
final String insertQuery = "insert into user (id, username, firstname, lastname) values (?, ?, ?, ?)";
KeyHolder keyHolder = new GeneratedKeyHolder();
jdbcTemplate.update(new PreparedStatementCreator() {
public PreparedStatement createPreparedStatement(Connection connection) throws SQLException {
PreparedStatement ps = connection.prepareStatement(insertQuery, new String[] { "id" });
ps.setInt(1, user.getUserId());
ps.setString(2, user.getUserName());
ps.setString(3, user.getFirstName());
ps.setInt(4, user.getLastName());
return ps;
}
}, keyHolder);
int userId = keyHolder.getKey().intValue();
return "user created successfully with user id: " + userId;
} catch (DataAccessException e) {
log.error(e, e);
}
}
FindBugs Issue:
FindBugs 问题：
Method may fail to clean up stream or resource on checked exception in this line PreparedStatement ps = connection.prepareStatement(insertQuery, new String[] { "id" });
方法可能无法清除此行中已检查异常的流或资源 PreparedStatement ps = connection.prepareStatement(insertQuery, new String[] { "id" });
Could some one please brief me what is this exactly? And how can we solve this?
有人可以告诉我这到底是什么吗？我们如何解决这个问题？
Help would be appreciated :)
帮助将不胜感激:)
回答by Tom G
PreparedStatementis a Closeableresource. However, it looks like the JDBC template is responsible for closing it -- so FindBugs likely has stumbled across a false-positive.
PreparedStatement是一种Closeable资源。但是，看起来 JDBC 模板负责关闭它——因此 FindBugs 可能偶然发现了误报。
回答by David Harkness
Yes, this looks like a false positive which the FindBugs team would like to hear about so they can tune this warning. They've added specific exceptions for third-party methods in other tests, and I expect this would be handled the same way. You can file a bug reportor email the team.
是的，这看起来像是 FindBugs 团队希望听到的误报，以便他们可以调整此警告。他们在其他测试中为第三方方法添加了特定的例外，我希望这会以相同的方式处理。您可以提交错误报告或向团队发送电子邮件。
For now, however, you can ignore this warning in this one case using the SuppressFBWarningsannotation:
但是，就目前而言，您可以使用SuppressFBWarnings注释在这种情况下忽略此警告：
@SuppressFBWarnings("OBL_UNSATISFIED_OBLIGATION_EXCEPTION_EDGE")
public PreparedStatement createPreparedStatement...
To improve readability and allow reusing warnings I found it helpful to define constants in a helper class:
为了提高可读性并允许重用警告，我发现在辅助类中定义常量很有帮助：
public final class FindBugs {
final String UNCLOSED_RESOURCE = "OBL_UNSATISFIED_OBLIGATION_EXCEPTION_EDGE";
private FindBugs() {
// static only
}
}
...
@SuppressFBWarnings(FindBugs.UNCLOSED_RESOURCE)
Unfortunately, I was not able to define an annotation that ignored a specific warning.
不幸的是，我无法定义忽略特定警告的注释。
回答by jmehrens
FindBugs is right about the potential leakon exception case because setIntand setStringare declared to throw 'SQLException'. If any of those lines were to throw a SQLException then the PreparedStatement is leaked because there is no scope block that has access to close it.
FindBugs关于异常情况的潜在泄漏是正确的，因为setInt和setString被声明为抛出“SQLException”。如果这些行中的任何一行抛出 SQLException，那么 PreparedStatement 就会泄漏，因为没有可以访问关闭它的作用域块。
To better understand this issue let's break down the code illusion by getting rid of the spring types and inline the method in way that is an approximation of how the callstack scoping would work when calling a method that returns a resource.
为了更好地理解这个问题，让我们通过摆脱 spring 类型并内联方法来分解代码错觉，这种方式近似于调用返回资源的方法时调用堆栈范围的工作方式。
public void leakyMethod(Connection con) throws SQLException {
PreparedStatement notAssignedOnThrow = null; //Simulate calling method storing the returned value.
try { //Start of what would be createPreparedStatement method
PreparedStatement inMethod = con.prepareStatement("select * from foo where key = ?");
//If we made it here a resource was allocated.
inMethod.setString(1, "foo"); //<--- This can throw which will skip next line.
notAssignedOnThrow = inMethod; //return from createPreparedStatement method call.
} finally {
if (notAssignedOnThrow != null) { //No way to close because it never
notAssignedOnThrow.close(); //made it out of the try block statement.
}
}
}
Going back to the original issue, the same is true if useris null resulting in a NullPointerExceptiondue to no user given or some other custom exception say UserNotLoggedInExceptionis thrown from deep inside of getUserId().
回到最初的问题，如果user为 null 导致NullPointerException由于没有给定用户或其他一些自定义异常UserNotLoggedInException从getUserId().
Here is an example of an uglyfix for this issue:
以下是针对此问题的丑陋修复示例：
public PreparedStatement createPreparedStatement(Connection connection) throws SQLException {
boolean fail = true;
PreparedStatement ps = connection.prepareStatement(insertQuery, new String[] { "id" });
try {
ps.setInt(1, user.getUserId());
ps.setString(2, user.getUserName());
ps.setString(3, user.getFirstName());
ps.setInt(4, user.getLastName());
fail = false;
} finally {
if (fail) {
try {
ps.close();
} catch(SQLException warn) {
}
}
}
return ps;
}
So in this example it only closes the statement if things have gone wrong. Otherwise return an open statement for the caller to clean up. A finally block is used over a catch block as a buggy driver implementation can throw more than just SQLException objects. Catch block and rethrow isn't used because inspecting type of a throwable can failin super rare cases.
所以在这个例子中，它只在出现问题时关闭语句。否则返回一个 open 语句供调用者清理。finally 块用于 catch 块，因为有缺陷的驱动程序实现可能抛出的不仅仅是 SQLException 对象。不使用捕获块和重新抛出，因为在极少数情况下检查可抛出的类型可能会失败。
In JDK 7 and JDK 8you can write the patch like this:
在JDK 7 和 JDK 8 中，您可以像这样编写补丁：
public PreparedStatement createPreparedStatement(Connection connection) throws SQLException {
PreparedStatement ps = connection.prepareStatement(insertQuery, new String[] { "id" });
try {
ps.setInt(1, user.getUserId());
ps.setString(2, user.getUserName());
ps.setString(3, user.getFirstName());
ps.setInt(4, user.getLastName());
} catch (Throwable t) {
try {
ps.close();
} catch (SQLException warn) {
if (t != warn) {
t.addSuppressed(warn);
}
}
throw t;
}
return ps;
}
In JDK 9 and lateryou can write the patch like this:
在JDK 9 及更高版本中，您可以像这样编写补丁：
public PreparedStatement createPreparedStatement(Connection connection) throws SQLException {
PreparedStatement ps = connection.prepareStatement(insertQuery, new String[] { "id" });
try {
ps.setInt(1, user.getUserId());
ps.setString(2, user.getUserName());
ps.setString(3, user.getFirstName());
ps.setInt(4, user.getLastName());
} catch (Throwable t) {
try (ps) { // closes statement on error
throw t;
}
}
return ps;
}
With regard to Spring, say your user.getUserId()method could throw IllegalStateException or the given user is null. Contractually, Spring does not specify what happens if java.lang.RuntimeException or java.lang.Error is thrownfrom a PreparedStatementCreator. Per the docs:
关于 Spring，假设您的user.getUserId()方法可能抛出 IllegalStateException 或给定的用户是null。按照约定，Spring 没有指定如果从 PreparedStatementCreator抛出 java.lang.RuntimeException 或 java.lang.Error会发生什么。根据文档：
Implementations do not need to concern themselves with SQLExceptions that may be thrown from operations they attempt. The JdbcTemplate class will catch and handle SQLExceptions appropriately.
实现不需要关心可能从他们尝试的操作中抛出的 SQLExceptions。JdbcTemplate 类将适当地捕获和处理 SQLExceptions。
That verbiage implies that Spring is relying on connection.close() doing the work.
这句话暗示 Spring 依赖connection.close() 来完成工作。
Let's make proof of concept to verify what the Spring documentation promises.
让我们进行概念验证以验证 Spring 文档所承诺的内容。
public class LeakByStackPop {
public static void main(String[] args) throws Exception {
Connection con = new Connection();
try {
PreparedStatement ps = createPreparedStatement(con);
try {
} finally {
ps.close();
}
} finally {
con.close();
}
}
static PreparedStatement createPreparedStatement(Connection connection) throws Exception {
PreparedStatement ps = connection.prepareStatement();
ps.setXXX(1, ""); //<---- Leak.
return ps;
}
private static class Connection {
private final PreparedStatement hidden = new PreparedStatement();
Connection() {
}
public PreparedStatement prepareStatement() {
return hidden;
}
public void close() throws Exception {
hidden.closeFromConnection();
}
}
private static class PreparedStatement {
public void setXXX(int i, String value) throws Exception {
throw new Exception();
}
public void close() {
System.out.println("Closed the statement.");
}
public void closeFromConnection() {
System.out.println("Connection closed the statement.");
}
}
}
The resulting output is:
结果输出是：
Connection closed the statement.
Exception in thread "main" java.lang.Exception
at LeakByStackPop$PreparedStatement.setXXX(LeakByStackPop.java:52)
at LeakByStackPop.createPreparedStatement(LeakByStackPop.java:28)
at LeakByStackPop.main(LeakByStackPop.java:15)
As you can see the connection is the only reference to the prepared statement.
如您所见，连接是对准备好的语句的唯一引用。
Let's update the example to fix the memory leak by patching our fake 'PreparedStatementCreator' method.
让我们通过修补我们的假“PreparedStatementCreator”方法来更新示例以修复内存泄漏。
public class LeakByStackPop {
public static void main(String[] args) throws Exception {
Connection con = new Connection();
try {
PreparedStatement ps = createPreparedStatement(con);
try {
} finally {
ps.close();
}
} finally {
con.close();
}
}
static PreparedStatement createPreparedStatement(Connection connection) throws Exception {
PreparedStatement ps = connection.prepareStatement();
try {
//If user.getUserId() could throw IllegalStateException
//when the user is not logged in then the same leak can occur.
ps.setXXX(1, "");
} catch (Throwable t) {
try {
ps.close();
} catch (Exception suppressed) {
if (suppressed != t) {
t.addSuppressed(suppressed);
}
}
throw t;
}
return ps;
}
private static class Connection {
private final PreparedStatement hidden = new PreparedStatement();
Connection() {
}
public PreparedStatement prepareStatement() {
return hidden;
}
public void close() throws Exception {
hidden.closeFromConnection();
}
}
private static class PreparedStatement {
public void setXXX(int i, String value) throws Exception {
throw new Exception();
}
public void close() {
System.out.println("Closed the statement.");
}
public void closeFromConnection() {
System.out.println("Connection closed the statement.");
}
}
}
The resulting output is:
结果输出是：
Closed the statement.
Exception in thread "main" java.lang.Exception
Connection closed the statement.
at LeakByStackPop$PreparedStatement.setXXX(LeakByStackPop.java:63)
at LeakByStackPop.createPreparedStatement(LeakByStackPop.java:29)
at LeakByStackPop.main(LeakByStackPop.java:15)
As you can see each allocation was balanced with a close to release the resource.
正如您所看到的，每个分配都通过 close 来平衡以释放资源。
回答by Nathan Hughes
Spring will close your PreparedStatement, that part is not a problem. Spring provided a way for you to pass in a callback that creates a PreparedStatement, Spring knows to close it once it's done. Specifically, the api doc for the PreparedStatementCreatorpromises the jdbcTemplate will close it:
Spring 会关闭你的 PreparedStatement，那部分不是问题。Spring 为您提供了一种方法来传入创建 PreparedStatement 的回调，Spring 知道一旦完成就关闭它。具体来说，PreparedStatementCreator的api 文档承诺 jdbcTemplate 将关闭它：
The JdbcTemplate will close the created statement.
JdbcTemplate 将关闭 created 语句。
Spring also will handle SQLExceptions, the same javadoc says:
Spring 也会处理 SQLExceptions，同一个 javadoc 说：
there is no need to catch SQLExceptions that may be thrown in the implementation of this method. The JdbcTemplate class will handle them.
不需要捕获在此方法的实现中可能抛出的 SQLExceptions。JdbcTemplate 类将处理它们。
Even through the JdbcTemplate class will handle the SQLExceptions, if the PreparedStatement throws a SQLException while setting a parameter, the prepared statement won't get closed by the jdbcTemplate code. But in that case you have worse problems than an unclosed PreparedStatement, you have a mismatched parameter.
即使通过 JdbcTemplate 类将处理 SQLExceptions，如果 PreparedStatement 在设置参数时抛出 SQLException，则准备好的语句不会被 jdbcTemplate 代码关闭。但是在那种情况下，您遇到的问题比未关闭的 PreparedStatement 更糟糕，您的参数不匹配。
If you examine the source code, the update method calls this execute method:
如果检查源代码，update 方法会调用此 execute 方法：
@Override
public <T> T [More ...] execute(PreparedStatementCreator psc, PreparedStatementCallback<T> action)
throws DataAccessException {
Assert.notNull(psc, "PreparedStatementCreator must not be null");
Assert.notNull(action, "Callback object must not be null");
if (logger.isDebugEnabled()) {
String sql = getSql(psc);
logger.debug("Executing prepared SQL statement" + (sql != null ? " [" + sql + "]" : ""));
}
Connection con = DataSourceUtils.getConnection(getDataSource());
PreparedStatement ps = null;
try {
Connection conToUse = con;
if (this.nativeJdbcExtractor != null && this.nativeJdbcExtractor.isNativeConnectionNecessaryForNativePreparedStatements()) {
conToUse = this.nativeJdbcExtractor.getNativeConnection(con);
}
ps = psc.createPreparedStatement(conToUse);
applyStatementSettings(ps);
PreparedStatement psToUse = ps;
if (this.nativeJdbcExtractor != null) {
psToUse = this.nativeJdbcExtractor.getNativePreparedStatement(ps);
}
T result = action.doInPreparedStatement(psToUse);
handleWarnings(ps);
return result;
}
catch (SQLException ex) {
// Release Connection early, to avoid potential connection pool deadlock
// in the case when the exception translator hasn't been initialized yet.
if (psc instanceof ParameterDisposer) {
((ParameterDisposer) psc).cleanupParameters();
}
String sql = getSql(psc);
psc = null;
JdbcUtils.closeStatement(ps);
ps = null;
DataSourceUtils.releaseConnection(con, getDataSource());
con = null;
throw getExceptionTranslator().translate("PreparedStatementCallback", sql, ex);
}
finally {
if (psc instanceof ParameterDisposer) {
((ParameterDisposer) psc).cleanupParameters();
}
JdbcUtils.closeStatement(ps);
DataSourceUtils.releaseConnection(con, getDataSource());
}
}
It would be unrealistic to expect static code analysis tools to be smart enough to get everything right, there's only so much they can do.
期望静态代码分析工具足够聪明以将一切都做好是不现实的，他们能做的只有这么多。
For me, the real issue with this code is where you catch and log the exception. Not letting the exception be thrown prevents Spring from rolling back the transaction when an error occurs. Either get rid of the try-catch and let the DataAccessException be thrown, or (if you must log it here) rethrow it after logging.
对我来说，这段代码的真正问题在于您在哪里捕获并记录异常。不让异常抛出可以防止 Spring 在发生错误时回滚事务。要么摆脱 try-catch 并抛出 DataAccessException，要么（如果您必须在此处记录它）在记录后重新抛出它。
