方法可能无法根据检查的异常清理流或资源-FindBugs [英] Method may fail to clean up stream or resource on checked exception -- FindBugs
问题描述
我正在使用Spring JDBCTemplate来访问数据库中的数据，并且它工作正常.但是FindBugs在代码片段中指出了我的一个小问题.
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.
代码:
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问题:
此行PreparedStatement ps = connection.prepareStatement(insertQuery, new String[] { "id" });
有人可以告诉我这到底是什么吗?而我们该如何解决呢?
Could some one please brief me what is this exactly? And how can we solve this?
我们将不胜感激:)
推荐答案
FindBugs is right about the potential leak on exception case because setInt and setString are 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.
为了更好地理解此问题，让我们摆脱掉弹簧类型来分解代码幻觉，并内联该方法，以近似于调用返回资源的方法时调用堆栈作用域的工作原理.
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.
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.
}
}
}
回到最初的问题，如果user为null导致NullPointerException由于没有给定用户或其他自定义异常而导致UserNotLoggedInException从getUserId()的深处抛出，则同样如此.
Going back to the original issue, the same is true if user is null resulting in a NullPointerException due to no user given or some other custom exception say UserNotLoggedInException is thrown from deep inside of getUserId().
以下是此问题的丑陋修复程序的示例:
Here is an example of an ugly fix 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;
}
因此，在此示例中，只有在出现问题时才关闭该语句.否则，返回打开的语句供调用方清除.由于错误的驱动程序实现不仅可以抛出SQLException对象，还可以在catch块上使用finally块.未使用捕获阻止和重新抛出，因为在极少数情况下检查抛出对象的类型可能会失败.
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 fail in super rare cases.
在 JDK 7和JDK 8 中，您可以编写如下补丁:
In JDK 7 and JDK 8 you can write the patch like this:
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;
}
在 JDK 9和更高版本中，您可以编写如下补丁:
In JDK 9 and later you can write the patch like this:
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;
}
关于Spring，假设您的user.getUserId()方法可能抛出IllegalStateException或给定的用户为null.根据合同，如果
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 thrown from a PreparedStatementCreator. Per the docs:
实现不必担心SQLException可能从尝试执行的操作中抛出. JdbcTemplate类将适当地捕获和处理SQLException.
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.
这种说法暗示着Spring依赖 connection.close()完成工作.
That verbiage implies that Spring is relying on connection.close() doing the work.
让我们通过概念验证来验证Spring文档的承诺.
Let's make proof of concept to verify what the Spring documentation promises.
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.");
}
}
}
结果输出为:
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.
让我们更新该示例，通过修补虚假的"PreparedStatementCreator"方法来修复内存泄漏.
Let's update the example to fix the memory leak by patching our fake 'PreparedStatementCreator' method.
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.");
}
}
}
结果输出为:
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.
这篇关于方法可能无法根据检查的异常清理流或资源-FindBugs的文章就介绍到这了，希望我们推荐的答案对大家有所帮助，也希望大家多多支持IT屋！
