- やす
- 会議室デビュー日: 2001/12/18
- 投稿数: 8
- お住まい・勤務地: 東京都
|
投稿日時: 2008-10-07 23:36
はじめまして。
Java6（1.6.0_01）でJDBC接続を使ったクライアントアプリを開発しています。
OSはWindowsXP、DBはOracle10gです。
皆さんのご意見・ご指摘をいただきたく投稿しました。
私は以下のように JDBC Type4 によるDB接続のソースコードを書いたのですが、
Eclipse3.3 にプラグインしている FindBugs によると、
「[ODR] メソッドは、例外が起きた時にデータベースリソースの解放に失敗するかもしれません。 」
という警告が2箇所に表示されます。
Eclipseを使った開発が初めてなので、FindBugsの癖などがあるのかもしれませんが、
私にはどこに危険が潜んでいるか見当がつきません。
皆さん、もしお分かりの方がいらっしゃいましたらぜひご指摘・ご教授いただけませんか。
| コード: |
| private static String executeScalar(final String sql, final String[] params) {
String result = null;
java.sql.Connection con = null;
java.sql.PreparedStatement pstmt = null;
java.sql.ResultSet rs = null;
try {
Class.forName("oracle.jdbc.driver.OracleDriver");
con = DriverManager.getConnection(DB_CONNECTION_STRING, USER_ID, USER_PASSWD); // この行にFindBugsの警告が出る。
pstmt = con.prepareStatement(sql); // この行にFindBugsの警告が出る。
if (params != null) {
for (int i = 0; i < params.length; i++)
pstmt.setString(i + 1, params[i]);
}
rs = pstmt.executeQuery();
if (rs.next())
result = rs.getString(1);
} catch (ClassNotFoundException cnfe) {
cnfe.printStackTrace();
// do nothing.
} catch (SQLException sqle) {
sqle.printStackTrace();
// do nothing.
} finally {
try {
if (rs != null) rs.close();
} catch (SQLException sqle) {
sqle.printStackTrace();
// do nothing.
}
try {
if (pstmt != null) pstmt.close();
} catch (SQLException sqle) {
sqle.printStackTrace();
// do nothing.
}
try {
if (con != null) con.close();
} catch (SQLException sqle) {
sqle.printStackTrace();
// do nothing.
}
}
return result;
}
|
＃余談（？）ですが。。
＃インターネット上に公開しているサンプルの多くは
＃ConnectionやStatementの解放をfinally句ではなく、
＃try句の中で行なっています。もちろん、サンプルですので
＃「参考程度にどうぞー」ということなのでしょうが、
＃あまりに多くのサンプルがこの書き方だったので、非常に衝撃的です。
＃（ずっと私は.NET開発が主だったので）Java流の作法があるのか？と
＃疑念を感じています。
| コード: |
| try {
con = DriverManager.getConnection(url, "myLogin", "myPassword");
stmt = con.createStatement();
（なんかの処理）
stmt.close();
con.close();
} catch(SQLException ex) {
System.err.print("SQLException: ");
System.err.println(ex.getMessage());
}
|
|
|---|
- かつのり
- ぬし
- 会議室デビュー日: 2004/03/18
- 投稿数: 2015
- お住まい・勤務地: 札幌
|
投稿日時: 2008-10-08 00:16
ぱっと見た感じ、全く問題がないように見えます。
FindBugsはバイトコードのパターンでバグ検出を行いますので、
似たコードパターンと間違っているかもしれませんね。
（この辺の詳細な誤検出の理由はわかりません）
| 引用: |
|
＃余談（？）ですが。。
＃インターネット上に公開しているサンプルの多くは
＃ConnectionやStatementの解放をfinally句ではなく、
＃try句の中で行なっています。もちろん、サンプルですので
＃「参考程度にどうぞー」ということなのでしょうが、
＃あまりに多くのサンプルがこの書き方だったので、非常に衝撃的です。
＃（ずっと私は.NET開発が主だったので）Java流の作法があるのか？と
＃疑念を感じています。
|
単に、サンプルが悪いようですね。
finallyを使ったリソースの解放は必要です。
|
|---|
- やす
- 会議室デビュー日: 2001/12/18
- 投稿数: 8
- お住まい・勤務地: 東京都
|
投稿日時: 2008-10-08 00:37
ご回答いただきありがとうございます。
Java言語が初めてだったので僅かに不安があったのですが
払拭できそうです。
| 引用: |
| 単に、サンプルが悪いようですね。
finallyを使ったリソースの解放は必要です。
|
そうですよね。DBの例外処理は言語に関係なくほぼ定石なのに
何故わざわざ省くのだろう。。（まぁ、公開者の自由ですが）
|
|---|
- holic
- ベテラン
- 会議室デビュー日: 2004/08/24
- 投稿数: 74
|
投稿日時: 2008-10-08 01:55
こう書けといっているだけでは？
finally 句の実行中に Exception が発生すると、FindBugs の主張はあたりますね。
| コード: |
|
java.sql.Connection con = null;
java.sql.PreparedStatement pstmt = null;
java.sql.ResultSet rs = null;
try {
Class.forName("oracle.jdbc.driver.OracleDriver");
con = DriverManager.getConnection(DB_CONNECTION_STRING, USER_ID,
USER_PASSWD);
try {
pstmt = con.prepareStatement(sql);
if (params != null) {
for (int i = 0; i < params.length; i++)
pstmt.setString(i + 1, params[i]);
}
try {
rs = pstmt.executeQuery();
if (rs.next())
result = rs.getString(1);
} catch (SQLException e) {
e.printStackTrace();
// do nothing.
} finally {
try {
if (rs != null)
rs.close();
} catch (SQLException sqle) {
sqle.printStackTrace();
// do nothing.
}
}
} catch (SQLException sqle) {
sqle.printStackTrace();
// do nothing.
} finally {
try {
if (pstmt != null)
pstmt.close();
} catch (SQLException sqle) {
sqle.printStackTrace();
// do nothing
}
}
} catch (ClassNotFoundException cnfe) {
cnfe.printStackTrace();
// do nothing.
} catch (SQLException sqle) {
sqle.printStackTrace();
// do nothing.
} finally {
try {
if (con != null)
con.close();
} catch (SQLException sqle) {
sqle.printStackTrace();
// do nothing.
}
}
return result;
|
|
|---|
- nagise
- ぬし
- 会議室デビュー日: 2006/05/19
- 投稿数: 1141
|
投稿日時: 2008-10-08 14:14
厳密にはfinallyのclose()3連発で例外が起きると解放が失敗するのでしょうけど、そこでSQLException以外の例外が発生する可能性を想定しないといけないのかどうなのか。
|
|---|
- かつのり
- ぬし
- 会議室デビュー日: 2004/03/18
- 投稿数: 2015
- お住まい・勤務地: 札幌
|
投稿日時: 2008-10-08 17:37
SQLException以外で、単純に考えられるのはNPEなんですが、
そこはチェックしていますからね・・・。
極端な話を言えば、Throwableをキャッチしないとダメですね。
ただ、SQLException以外がスローされるケースは、
そもそももっと致命的なケースですので、
アプリが継続可能な状態とは思えませんし、
自分の意見としては、十分な例外処理に思えます。
|
|---|
- やす
- 会議室デビュー日: 2001/12/18
- 投稿数: 8
- お住まい・勤務地: 東京都
|
投稿日時: 2008-10-09 00:23
| 引用: |
| finally 句の実行中に Exception が発生すると、FindBugs の主張はあたりますね。
|
なるほど。確かに finally 句の中で SQLExceptioin 以外の例外が発生する可能性は（限りなくゼロだと感じますが）、はっきりゼロと断言することはできません。また、各オブジェクトのスコープや生存期間を必要最小限にとどめるという意味ではむしろ賛同します。
| 引用: |
| 厳密にはfinallyのclose()3連発で例外が起きると解放が失敗するのでしょうけど、そこでSQLException以外の例外が発生する可能性を想定しないといけないのかどうなのか。
|
私の意見としては、各クラスやメソッドの役割・責務を超えた制御（特に例外伝播の握りつぶし）はするべきではないと考えています。
おそらく FindBugs はそこまでの意図は無いのではないかと感じます。つまり、「あらゆる可能性を考慮した際に、リソースを解放しないルートが存在している」ことを単純に示しているに過ぎないのではないでしょうか。
皆さんのご意見から、私はコードを以下のように書き換えて見ました。
| コード: |
| private static String executeScalar(final String select_sql, final String[] params) {
String result = null;
java.sql.Connection con = null;
java.sql.PreparedStatement pstmt = null;
java.sql.ResultSet rs = null;
try {
Class.forName(DRIVER_NAME);
con = DriverManager.getConnection(DB_CONNECTION_STRING, USER_ID, USER_PASSWD);
try {
pstmt = con.prepareStatement(select_sql);
if (params != null) {
for (int i = 0; i < params.length; i++) {
pstmt.setString(i + 1, params[i]);
}
}
try {
rs = pstmt.executeQuery();
if (rs.next())
result = rs.getString(1);
} finally {
try {
if (rs != null) rs.close();
} catch (SQLException sqle) {
sqle.printStackTrace();
// do nothing.
}
}
} finally {
try {
if (pstmt != null) pstmt.close();
} catch (SQLException sqle) {
sqle.printStackTrace();
// do nothing.
}
}
} catch (ClassNotFoundException cnfe) {
cnfe.printStackTrace();
// do nothing.
} catch (SQLException sqle) {
sqle.printStackTrace();
// do nothing.
} finally {
try {
if (con != null) con.close();
} catch (SQLException sqle) {
sqle.printStackTrace();
// do nothing.
}
}
return result;
}
|
この書き方だと、FindBugs は警告を引っ込めました。（holicさんの書き込みと意味的にはまったく同じです。）
これならば、確かにどこで何の例外が発生してもリソースが解放されるルートは担保されています。
ただ、ここまで書くかどうかは、コードを書く開発者の趣向やコードの重要性に委ねられるでしょうね。
|
|---|
