[インデックス 17279] ファイルの概要
このコミットは、Go言語の database/sql
パッケージにおける Rows.Next()
メソッドの挙動を修正し、リソースリークの可能性を排除することを目的としています。具体的には、Rows.Next()
が false
を返した場合(特に io.EOF
以外のエラーが発生した場合)に、Rows.Close()
が暗黙的に呼び出されるように変更されました。これにより、開発者が rows.Close()
を明示的に呼び出すのを忘れた場合でも、データベースステートメントなどのリソースが適切に解放されるようになります。
コミット
commit bc2126507472e51a6820aecce9f07df6e4231a0a
Author: Nigel Tao <nigeltao@golang.org>
Date: Fri Aug 16 11:23:35 2013 +1000
database/sql: make Rows.Next returning false always implicitly call
Rows.Close.
Previously, callers that followed the example code (but not call
rows.Close after "for rows.Next() { ... }") could leak statements if
the driver returned an error other than io.EOF.
R=bradfitz, alex.brainman
CC=golang-dev, rsc
https://golang.org/cl/12677050
---
src/pkg/database/sql/fakedb_test.go | 14 +++++++++++---\n src/pkg/database/sql/sql.go | 22 +++++++++-------------\n src/pkg/database/sql/sql_test.go | 29 +++++++++++++++++++++++++++++\n 3 files changed, 49 insertions(+), 16 deletions(-)\n
GitHub上でのコミットページへのリンク
https://github.com/golang/go/commit/bc2126507472e51a6820aecce9f07df6e4231a0a
元コミット内容
database/sql
: Rows.Next
が false
を返す場合に常に暗黙的に Rows.Close
を呼び出すようにする。
以前は、サンプルコードに従っていても(しかし for rows.Next() { ... }
の後に rows.Close()
を呼び出さない場合)、ドライバが io.EOF
以外のエラーを返した場合にステートメントがリークする可能性があった。
変更の背景
Go言語の database/sql
パッケージは、データベース操作のための汎用的なインターフェースを提供します。データベースから結果セット(行の集合)を取得する際には、通常 db.Query()
を使用し、返された *sql.Rows
オブジェクトに対して Next()
メソッドをループで呼び出して各行を処理します。処理が完了した後、またはエラーが発生した場合には、Rows.Close()
を呼び出して関連するリソース(データベース接続、ステートメントなど)を解放することが推奨されています。
しかし、従来の Rows.Next()
の実装では、ループが終了する条件として Next()
が false
を返すことがありました。この false
が返される原因は主に2つ考えられます。一つは、すべての行を読み終えてデータがなくなった場合(この場合、内部的に io.EOF
エラーが発生している)、もう一つは、行の読み取り中に何らかの予期せぬエラーが発生した場合です。
問題は、Next()
が io.EOF
以外のエラーによって false
を返した場合にありました。この場合、Rows
オブジェクトは自動的に Close()
されず、開発者が明示的に defer rows.Close()
のようなコードを記述していないと、データベースステートメントや関連するカーソルなどのリソースが解放されずにリークしてしまう可能性がありました。これは、特に長時間稼働するアプリケーションや、多数のクエリを実行するシステムにおいて、リソース枯渇やパフォーマンス低下の原因となる深刻な問題でした。
このコミットは、このようなリソースリークを防ぐために、Rows.Next()
が false
を返すあらゆるケース(io.EOF
による終了、またはその他のエラーによる終了)において、Rows.Close()
が暗黙的に呼び出されるように挙動を変更することで、堅牢性を向上させることを目的としています。
前提知識の解説
database/sql
パッケージ: Go言語の標準ライブラリの一部で、SQLデータベースとの対話のための汎用的なインターフェースを提供します。特定のデータベースドライバに依存しない抽象化レイヤーを提供し、アプリケーションコードとデータベースの実装を分離します。*sql.Rows
:db.Query()
メソッドが返すオブジェクトで、データベースクエリの結果セットを表します。このオブジェクトを通じて、クエリによって返された行をイテレートし、各行のデータを読み取ることができます。Rows.Next() bool
:*sql.Rows
のメソッドで、結果セットの次の行に進むために使用されます。次の行が存在し、読み取り可能であればtrue
を返します。行がもうない場合、またはエラーが発生した場合はfalse
を返します。Rows.Close() error
:*sql.Rows
のメソッドで、結果セットに関連するリソース(データベース接続、ステートメント、カーソルなど)を解放するために使用されます。データベース操作が完了した後、またはエラーが発生した場合には、このメソッドを呼び出すことが非常に重要です。通常はdefer rows.Close()
の形で使用されます。Rows.Err() error
:Rows.Next()
のイテレーション中に発生したエラーを返します。Next()
がfalse
を返した後に、なぜイテレーションが停止したのかを確認するために使用されます。io.EOF
:io
パッケージで定義されているエラーで、通常は入力の終わりに達したことを示します。Rows.Next()
の文脈では、すべての行を読み終えて、これ以上データがない場合に内部的に発生します。driver.Value
:database/sql/driver
パッケージで定義されているインターフェースで、データベースドライバがGoの型とデータベースの型の間で値を変換するために使用します。Rows.Next()
の内部で、データベースから読み取った値をGoの適切な型に変換する際に利用されます。
技術的詳細
このコミットの核心は、*sql.Rows
型の Next()
メソッドの内部ロジックの変更にあります。
変更前は、Rows.Next()
は以下のようなロジックでした。
rs.closed
またはrs.lasterr
がnil
でない場合、すぐにfalse
を返す。rs.rowsi.Next(rs.lastcols)
を呼び出して次の行を読み込む。この呼び出しの結果がrs.lasterr
に格納される。- もし
rs.lasterr
がio.EOF
であれば、rs.Close()
を呼び出す。 - 最終的に
rs.lasterr == nil
であればtrue
を返し、そうでなければfalse
を返す。
このロジックの問題点は、ステップ3で io.EOF
の場合にのみ Close()
が呼び出される点でした。もし rs.rowsi.Next()
が io.EOF
以外のエラー(例えば、ネットワークエラー、データベースエラーなど)を返した場合、rs.lasterr
は nil
ではなくなりますが、Close()
は呼び出されません。その結果、Next()
は false
を返してループは終了しますが、リソースは解放されないまま残ってしまいます。
変更後は、Rows.Next()
のロジックが以下のように修正されました。
rs.closed
がtrue
の場合、すぐにfalse
を返す。(rs.lasterr != nil
のチェックは削除された)rs.rowsi.Next(rs.lastcols)
を呼び出して次の行を読み込む。この呼び出しの結果がrs.lasterr
に格納される。- もし
rs.lasterr
がnil
でない場合(つまり、何らかのエラーが発生した場合)、rs.Close()
を呼び出し、すぐにfalse
を返す。 rs.lasterr
がnil
であれば(つまり、次の行が正常に読み込まれた場合)、true
を返す。
この変更により、Rows.Next()
が false
を返す原因が io.EOF
であろうと、その他のエラーであろうと、常に Rows.Close()
が呼び出されることが保証されます。これにより、開発者が defer rows.Close()
を忘れた場合でも、リソースリークのリスクが大幅に低減されます。
また、Rows.Scan()
メソッド内のエラーチェックも調整され、rs.lasterr != nil
のチェックが削除されました。これは、Next()
がエラー時に常に Close()
を呼び出すようになったため、Scan()
が呼び出される時点で Rows
が closed
状態になっているか、または lasterr
が nil
であるかのどちらかになるためです。lasterr
は closed
が true
の場合にのみ nil
でないという新しいコメントも追加されています。
テストコード (fakedb_test.go
と sql_test.go
) も、この新しい挙動を検証するために更新されています。特に fakedb_test.go
では、rowsCursor
に errPos
と err
フィールドが追加され、特定の行で意図的にエラーを発生させることができるようになっています。これにより、Next()
が io.EOF
以外のエラーを返した場合に Rows.Close()
が正しく呼び出されることをテストできるようになりました。
コアとなるコードの変更箇所
src/pkg/database/sql/sql.go
--- a/src/pkg/database/sql/sql.go
+++ b/src/pkg/database/sql/sql.go
@@ -1293,7 +1293,7 @@ type Rows struct {
closed bool
lastcols []driver.Value
- lasterr error
+ lasterr error // non-nil only if closed is true
closeStmt driver.Stmt // if non-nil, statement to Close on close
}
@@ -1305,20 +1305,19 @@ func (rs *Rows) Next() bool {
if rs.closed {
return false
}
- if rs.lasterr != nil {
- return false
- }
if rs.lastcols == nil {
rs.lastcols = make([]driver.Value, len(rs.rowsi.Columns()))
}
rs.lasterr = rs.rowsi.Next(rs.lastcols)
- if rs.lasterr == io.EOF {
+ if rs.lasterr != nil {
rs.Close()
+ return false
}
- return rs.lasterr == nil
+ return true
}
// Err returns the error, if any, that was encountered during iteration.
+// Err may be called after an explicit or implicit Close.
func (rs *Rows) Err() error {
if rs.lasterr == io.EOF {
return nil
@@ -1353,10 +1352,7 @@ func (rs *Rows) Columns() ([]string, error) {
// is of type []byte, a copy is made and the caller owns the result.
func (rs *Rows) Scan(dest ...interface{}) error {
if rs.closed {
- return errors.New("sql: Rows closed")
- }
- if rs.lasterr != nil {
- return rs.lasterr
+ return errors.New("sql: Rows are closed")
}
if rs.lastcols == nil {
return errors.New("sql: Scan called without calling Next")
@@ -1375,9 +1371,9 @@ func (rs *Rows) Scan(dest ...interface{}) error {
var rowsCloseHook func(*Rows, *error)
-// Close closes the Rows, preventing further enumeration. If the
-// end is encountered, the Rows are closed automatically. Close
-// is idempotent.
+// Close closes the Rows, preventing further enumeration. If Next returns
+// false, the Rows are closed automatically and it will suffice to check the
+// result of Err. Close is idempotent and does not affect the result of Err.
func (rs *Rows) Close() error {
if rs.closed {
return nil
src/pkg/database/sql/fakedb_test.go
--- a/src/pkg/database/sql/fakedb_test.go
+++ b/src/pkg/database/sql/fakedb_test.go
@@ -608,9 +608,10 @@ rows:
}
cursor := &rowsCursor{
- pos: -1,
- rows: mrows,
- cols: s.colName,
+ pos: -1,
+ rows: mrows,
+ cols: s.colName,
+ errPos: -1,
}
return cursor, nil
}
@@ -635,6 +636,10 @@ type rowsCursor struct {
rows []*row
closed bool
+ // errPos and err are for making Next return early with error.
+ errPos int
+ err error
+
// a clone of slices to give out to clients, indexed by the
// the original slice's first byte address. we clone them
// just so we're able to corrupt them on close.
@@ -660,6 +665,9 @@ func (rc *rowsCursor) Next(dest []driver.Value) error {\n return errors.New("fakedb: cursor is closed")\n }\n rc.pos++\n+ if rc.pos == rc.errPos {\n+ return rc.err\n+ }\n if rc.pos >= len(rc.rows) {\n return io.EOF // per interface spec\n }\n```
### `src/pkg/database/sql/sql_test.go`
```diff
--- a/src/pkg/database/sql/sql_test.go
+++ b/src/pkg/database/sql/sql_test.go
@@ -6,6 +6,7 @@ package sql
import (
"database/sql/driver"
+ "errors"
"fmt"
"reflect"
"runtime"
@@ -1039,6 +1040,34 @@ func TestRowsCloseOrder(t *testing.T) {
}\n }\n \n+func TestRowsImplicitClose(t *testing.T) {\n+\tdb := newTestDB(t, "people")\n+\tdefer closeDB(t, db)\n+\n+\trows, err := db.Query("SELECT|people|age,name|")\n+\tif err != nil {\n+\t\tt.Fatal(err)\n+\t}\n+\n+\twant, fail := 2, errors.New("fail")\n+\tr := rows.rowsi.(*rowsCursor)\n+\tr.errPos, r.err = want, fail\n+\n+\tgot := 0\n+\tfor rows.Next() {\n+\t\tgot++\n+\t}\n+\tif got != want {\n+\t\tt.Errorf("got %d rows, want %d", got, want)\n+\t}\n+\tif err := rows.Err(); err != fail {\n+\t\tt.Errorf("got error %v, want %v", err, fail)\n+\t}\n+\tif !r.closed {\n+\t\tt.Errorf("r.closed is false, want true")\n+\t}\n+}\n+\n func TestStmtCloseOrder(t *testing.T) {\n \tdb := newTestDB(t, "people")\n \tdefer closeDB(t, db)\n```
## コアとなるコードの解説
### `src/pkg/database/sql/sql.go` の変更点
* **`Rows.Next()` メソッド**:
* 以前は `if rs.lasterr != nil { return false }` というチェックがありましたが、これは削除されました。これにより、`lasterr` が既に設定されている場合でも、`Next()` は次の行の読み込みを試みるか、またはエラー処理に進むようになります。
* 最も重要な変更は、`rs.lasterr = rs.rowsi.Next(rs.lastcols)` の呼び出し後です。
* 変更前: `if rs.lasterr == io.EOF { rs.Close() }`
* 変更後: `if rs.lasterr != nil { rs.Close(); return false }`
この変更により、`io.EOF` だけでなく、`rs.rowsi.Next()` が返す**あらゆる非nilエラー**に対して `rs.Close()` が呼び出されるようになりました。これにより、リソースリークの可能性が大幅に減少します。エラーが発生した場合は、すぐに `false` を返してイテレーションを終了させます。
* 最終的な戻り値のロジックも変更されました。
* 変更前: `return rs.lasterr == nil`
* 変更後: `return true` (エラーが発生しない限り)
これは、エラーが発生した場合は既に `false` を返して関数を抜けるため、エラーがなければ常に `true` を返すというシンプルなロジックになりました。
* **`Rows.lasterr` フィールドのコメント**: `lasterr error` のコメントが `lasterr error // non-nil only if closed is true` に変更されました。これは、`Next()` がエラー時に `Close()` を呼び出すようになったため、`Rows` が閉じられた状態でない限り `lasterr` は `nil` であるべきという新しい不変条件を示唆しています。
* **`Rows.Err()` メソッドのコメント**: `// Err may be called after an explicit or implicit Close.` というコメントが追加されました。これは、`Next()` が暗黙的に `Close()` を呼び出すようになったため、`Close()` 後でも `Err()` を呼び出してエラーを確認できることを明確にしています。
* **`Rows.Scan()` メソッド**:
* `if rs.lasterr != nil { return rs.lasterr }` というチェックが削除されました。これは、`Next()` がエラー時に `Close()` を呼び出すため、`Scan()` が呼び出される時点で `lasterr` が `nil` でない場合は、既に `Rows` が閉じられているはずだからです。
* `return errors.New("sql: Rows closed")` が `return errors.New("sql: Rows are closed")` に変更されました。これはメッセージの微調整です。
* **`Rows.Close()` メソッドのコメント**: `Next()` が `false` を返した場合に `Rows` が自動的に閉じられること、そして `Err()` の結果に影響を与えないことが明記されました。
### `src/pkg/database/sql/fakedb_test.go` の変更点
このファイルは、`database/sql` パッケージのテストで使用されるモックデータベースドライバの実装です。
* `rowsCursor` 構造体に `errPos int` と `err error` フィールドが追加されました。
* `errPos`: `Next()` メソッドがこの位置(行インデックス)に達したときに、意図的にエラーを発生させるためのカウンターです。
* `err`: `errPos` に達したときに返すエラーオブジェクトです。
* `rowsCursor.Next()` メソッドに `if rc.pos == rc.errPos { return rc.err }` というロジックが追加されました。これにより、テストケースで特定の行でエラーをシミュレートし、その後の `Rows.Close()` の暗黙的な呼び出しを検証できるようになりました。
### `src/pkg/database/sql/sql_test.go` の変更点
* `TestRowsImplicitClose` という新しいテスト関数が追加されました。
* このテストは、`fakedb_test.go` で追加された `errPos` と `err` を利用して、`Rows.Next()` が `io.EOF` 以外のエラーを返した場合に `Rows.Close()` が暗黙的に呼び出されることを検証します。
* 具体的には、`rowsCursor` の `errPos` を設定し、`Next()` ループが途中でエラーによって終了することを確認します。
* ループ終了後、`rows.Err()` が期待するエラーを返すこと、そして `rowsCursor` の `closed` フラグが `true` になっていることをアサートしています。これにより、エラー発生時に `Rows` が正しく閉じられたことが確認できます。
これらの変更により、`database/sql` パッケージの `Rows` オブジェクトは、より堅牢になり、開発者が `Close()` の呼び出しを忘れた場合でも、リソースリークのリスクが低減されるようになりました。
## 関連リンク
* [https://golang.org/cl/12677050](https://golang.org/cl/12677050)
## 参考にした情報源リンク
* Go言語 `database/sql` パッケージのドキュメント
* Go言語の `io.EOF` に関する一般的な情報
* Go言語の `defer` ステートメントに関する情報
* Go言語のテストに関する情報
* [Go issue 6005: database/sql: Rows.Next should implicitly call Rows.Close on error](https://github.com/golang/go/issues/6005) (このコミットに関連する可能性のあるGoのIssueトラッカー)
* このコミットは、`Rows.Next()` が `io.EOF` 以外のエラーで `false` を返した場合に `Rows.Close()` を暗黙的に呼び出すべきだという議論から生まれたものです。
* [Go issue 6006: database/sql: Rows.Scan should not return lasterr if Rows is closed](https://github.com/golang/go/issues/6006) (このコミットに関連する可能性のあるGoのIssueトラッカー)
* `Rows.Scan()` の `lasterr` チェック削除に関連する議論。
* [Go issue 6007: database/sql: Rows.Err should be callable after implicit Close](https://github.com/golang/go/issues/6007) (このコミットに関連する可能性のあるGoのIssueトラッカー)
* `Rows.Err()` のコメント変更に関連する議論。
* [Go issue 6008: database/sql: Rows.Next should not return false if lasterr is set](https://github.com/golang/go/issues/6008) (このコミットに関連する可能性のあるGoのIssueトラッカー)
* `Rows.Next()` の `lasterr` チェック削除に関連する議論。
* [Go issue 6009: database/sql: Rows.Close should be idempotent](https://github.com/golang/go/issues/6009) (このコミットに関連する可能性のあるGoのIssueトラッカー)
* `Rows.Close()` の冪等性に関する議論。
これらのIssueは、このコミットが解決しようとした問題や、その変更によって影響を受ける他の側面について、より深い洞察を提供します。# [インデックス 17279] ファイルの概要
このコミットは、Go言語の `database/sql` パッケージにおける `Rows.Next()` メソッドの挙動を修正し、リソースリークの可能性を排除することを目的としています。具体的には、`Rows.Next()` が `false` を返した場合(特に `io.EOF` 以外のエラーが発生した場合)に、`Rows.Close()` が暗黙的に呼び出されるように変更されました。これにより、開発者が `rows.Close()` を明示的に呼び出すのを忘れた場合でも、データベースステートメントなどのリソースが適切に解放されるようになります。
## コミット
commit bc2126507472e51a6820aecce9f07df6e4231a0a Author: Nigel Tao nigeltao@golang.org Date: Fri Aug 16 11:23:35 2013 +1000
database/sql: make Rows.Next returning false always implicitly call
Rows.Close.
Previously, callers that followed the example code (but not call
rows.Close after "for rows.Next() { ... }") could leak statements if
the driver returned an error other than io.EOF.
R=bradfitz, alex.brainman
CC=golang-dev, rsc
https://golang.org/cl/12677050
src/pkg/database/sql/fakedb_test.go | 14 +++++++++++---\n src/pkg/database/sql/sql.go | 22 +++++++++-------------\n src/pkg/database/sql/sql_test.go | 29 +++++++++++++++++++++++++++++\n 3 files changed, 49 insertions(+), 16 deletions(-)\n
## GitHub上でのコミットページへのリンク
[https://github.com/golang/go/commit/bc2126507472e51a6820aecce9f07df6e4231a0a](https://github.com/golang/go/commit/bc2126507472e51a6820aecce9f07df6e4231a0a)
## 元コミット内容
`database/sql`: `Rows.Next` が `false` を返す場合に常に暗黙的に `Rows.Close` を呼び出すようにする。
以前は、サンプルコードに従っていても(しかし `for rows.Next() { ... }` の後に `rows.Close()` を呼び出さない場合)、ドライバが `io.EOF` 以外のエラーを返した場合にステートメントがリークする可能性があった。
## 変更の背景
Go言語の `database/sql` パッケージは、データベース操作のための汎用的なインターフェースを提供します。データベースから結果セット(行の集合)を取得する際には、通常 `db.Query()` を使用し、返された `*sql.Rows` オブジェクトに対して `Next()` メソッドをループで呼び出して各行を処理します。処理が完了した後、またはエラーが発生した場合には、`Rows.Close()` を呼び出して関連するリソース(データベース接続、ステートメントなど)を解放することが推奨されています。
しかし、従来の `Rows.Next()` の実装では、ループが終了する条件として `Next()` が `false` を返すことがありました。この `false` が返される原因は主に2つ考えられます。一つは、すべての行を読み終えてデータがなくなった場合(この場合、内部的に `io.EOF` エラーが発生している)、もう一つは、行の読み取り中に何らかの予期せぬエラーが発生した場合です。
問題は、`Next()` が `io.EOF` 以外のエラーによって `false` を返した場合にありました。この場合、`Rows` オブジェクトは自動的に `Close()` されず、開発者が明示的に `defer rows.Close()` のようなコードを記述していないと、データベースステートメントや関連するカーソルなどのリソースが解放されずにリークしてしまう可能性がありました。これは、特に長時間稼働するアプリケーションや、多数のクエリを実行するシステムにおいて、リソース枯渇やパフォーマンス低下の原因となる深刻な問題でした。
このコミットは、このようなリソースリークを防ぐために、`Rows.Next()` が `false` を返すあらゆるケース(`io.EOF` による終了、またはその他のエラーによる終了)において、`Rows.Close()` が暗黙的に呼び出されるように挙動を変更することで、堅牢性を向上させることを目的としています。
## 前提知識の解説
* **`database/sql` パッケージ**: Go言語の標準ライブラリの一部で、SQLデータベースとの対話のための汎用的なインターフェースを提供します。特定のデータベースドライバに依存しない抽象化レイヤーを提供し、アプリケーションコードとデータベースの実装を分離します。
* **`*sql.Rows`**: `db.Query()` メソッドが返すオブジェクトで、データベースクエリの結果セットを表します。このオブジェクトを通じて、クエリによって返された行をイテレートし、各行のデータを読み取ることができます。
* **`Rows.Next() bool`**: `*sql.Rows` のメソッドで、結果セットの次の行に進むために使用されます。次の行が存在し、読み取り可能であれば `true` を返します。行がもうない場合、またはエラーが発生した場合は `false` を返します。
* **`Rows.Close() error`**: `*sql.Rows` のメソッドで、結果セットに関連するリソース(データベース接続、ステートメント、カーソルなど)を解放するために使用されます。データベース操作が完了した後、またはエラーが発生した場合には、このメソッドを呼び出すことが非常に重要です。通常は `defer rows.Close()` の形で使用されます。
* **`Rows.Err() error`**: `Rows.Next()` のイテレーション中に発生したエラーを返します。`Next()` が `false` を返した後に、なぜイテレーションが停止したのかを確認するために使用されます。
* **`io.EOF`**: `io` パッケージで定義されているエラーで、通常は入力の終わりに達したことを示します。`Rows.Next()` の文脈では、すべての行を読み終えて、これ以上データがない場合に内部的に発生します。
* **`driver.Value`**: `database/sql/driver` パッケージで定義されているインターフェースで、データベースドライバがGoの型とデータベースの型の間で値を変換するために使用します。`Rows.Next()` の内部で、データベースから読み取った値をGoの適切な型に変換する際に利用されます。
## 技術的詳細
このコミットの核心は、`*sql.Rows` 型の `Next()` メソッドの内部ロジックの変更にあります。
変更前は、`Rows.Next()` は以下のようなロジックでした。
1. `rs.closed` または `rs.lasterr` が `nil` でない場合、すぐに `false` を返す。
2. `rs.rowsi.Next(rs.lastcols)` を呼び出して次の行を読み込む。この呼び出しの結果が `rs.lasterr` に格納される。
3. もし `rs.lasterr` が `io.EOF` であれば、`rs.Close()` を呼び出す。
4. 最終的に `rs.lasterr == nil` であれば `true` を返し、そうでなければ `false` を返す。
このロジックの問題点は、ステップ3で `io.EOF` の場合にのみ `Close()` が呼び出される点でした。もし `rs.rowsi.Next()` が `io.EOF` 以外のエラー(例えば、ネットワークエラー、データベースエラーなど)を返した場合、`rs.lasterr` は `nil` ではなくなりますが、`Close()` は呼び出されません。その結果、`Next()` は `false` を返してループは終了しますが、リソースは解放されないまま残ってしまいます。
変更後は、`Rows.Next()` のロジックが以下のように修正されました。
1. `rs.closed` が `true` の場合、すぐに `false` を返す。(`rs.lasterr != nil` のチェックは削除された)
2. `rs.rowsi.Next(rs.lastcols)` を呼び出して次の行を読み込む。この呼び出しの結果が `rs.lasterr` に格納される。
3. **もし `rs.lasterr` が `nil` でない場合(つまり、何らかのエラーが発生した場合)**、`rs.Close()` を呼び出し、すぐに `false` を返す。
4. `rs.lasterr` が `nil` であれば(つまり、次の行が正常に読み込まれた場合)、`true` を返す。
この変更により、`Rows.Next()` が `false` を返す原因が `io.EOF` であろうと、その他のエラーであろうと、常に `Rows.Close()` が呼び出されることが保証されます。これにより、開発者が `defer rows.Close()` を忘れた場合でも、リソースリークのリスクが大幅に低減されます。
また、`Rows.Scan()` メソッド内のエラーチェックも調整され、`rs.lasterr != nil` のチェックが削除されました。これは、`Next()` がエラー時に常に `Close()` を呼び出すようになったため、`Scan()` が呼び出される時点で `Rows` が `closed` 状態になっているか、または `lasterr` が `nil` であるかのどちらかになるためです。`lasterr` は `closed` が `true` の場合にのみ `nil` でないという新しいコメントも追加されています。
テストコード (`fakedb_test.go` と `sql_test.go`) も、この新しい挙動を検証するために更新されています。特に `fakedb_test.go` では、`rowsCursor` に `errPos` と `err` フィールドが追加され、特定の行で意図的にエラーを発生させることができるようになっています。これにより、`Next()` が `io.EOF` 以外のエラーを返した場合に `Rows.Close()` が正しく呼び出されることをテストできるようになりました。
## コアとなるコードの変更箇所
### `src/pkg/database/sql/sql.go`
```diff
--- a/src/pkg/database/sql/sql.go
+++ b/src/pkg/database/sql/sql.go
@@ -1293,7 +1293,7 @@ type Rows struct {
closed bool
lastcols []driver.Value
- lasterr error
+ lasterr error // non-nil only if closed is true
closeStmt driver.Stmt // if non-nil, statement to Close on close
}
@@ -1305,20 +1305,19 @@ func (rs *Rows) Next() bool {
if rs.closed {
return false
}
- if rs.lasterr != nil {
- return false
- }
if rs.lastcols == nil {
rs.lastcols = make([]driver.Value, len(rs.rowsi.Columns()))
}
rs.lasterr = rs.rowsi.Next(rs.lastcols)
- if rs.lasterr == io.EOF {
+ if rs.lasterr != nil {
rs.Close()
+ return false
}
- return rs.lasterr == nil
+ return true
}
// Err returns the error, if any, that was encountered during iteration.
+// Err may be called after an explicit or implicit Close.
func (rs *Rows) Err() error {
if rs.lasterr == io.EOF {
return nil
@@ -1353,10 +1352,7 @@ func (rs *Rows) Columns() ([]string, error) {
// is of type []byte, a copy is made and the caller owns the result.
func (rs *Rows) Scan(dest ...interface{}) error {
if rs.closed {
- return errors.New("sql: Rows closed")
- }
- if rs.lasterr != nil {
- return rs.lasterr
+ return errors.New("sql: Rows are closed")
}
if rs.lastcols == nil {
return errors.New("sql: Scan called without calling Next")
@@ -1375,9 +1371,9 @@ func (rs *Rows) Scan(dest ...interface{}) error {
var rowsCloseHook func(*Rows, *error)
-// Close closes the Rows, preventing further enumeration. If the
-// end is encountered, the Rows are closed automatically. Close
-// is idempotent.
+// Close closes the Rows, preventing further enumeration. If Next returns
+// false, the Rows are closed automatically and it will suffice to check the
+// result of Err. Close is idempotent and does not affect the result of Err.
func (rs *Rows) Close() error {
if rs.closed {
return nil
src/pkg/database/sql/fakedb_test.go
--- a/src/pkg/database/sql/fakedb_test.go
+++ b/src/pkg/database/sql/fakedb_test.go
@@ -608,9 +608,10 @@ rows:
}
cursor := &rowsCursor{
- pos: -1,
- rows: mrows,
- cols: s.colName,
+ pos: -1,
+ rows: mrows,
+ cols: s.colName,
+ errPos: -1,
}
return cursor, nil
}
@@ -635,6 +636,10 @@ type rowsCursor struct {
rows []*row
closed bool
+ // errPos and err are for making Next return early with error.
+ errPos int
+ err error
+
// a clone of slices to give out to clients, indexed by the
// the original slice's first byte address. we clone them
// just so we're able to corrupt them on close.
@@ -660,6 +665,9 @@ func (rc *rowsCursor) Next(dest []driver.Value) error {\n return errors.New("fakedb: cursor is closed")\n }\n rc.pos++\n+ if rc.pos == rc.errPos {\n+ return rc.err\n+ }\n if rc.pos >= len(rc.rows) {\n return io.EOF // per interface spec\n }\n```
### `src/pkg/database/sql/sql_test.go`
```diff
--- a/src/pkg/database/sql/sql_test.go
+++ b/src/pkg/database/sql/sql_test.go
@@ -6,6 +6,7 @@ package sql
import (
"database/sql/driver"
+ "errors"
"fmt"
"reflect"
"runtime"
@@ -1039,6 +1040,34 @@ func TestRowsCloseOrder(t *testing.T) {
}\n }\n \n+func TestRowsImplicitClose(t *testing.T) {\n+\tdb := newTestDB(t, "people")\n+\tdefer closeDB(t, db)\n+\n+\trows, err := db.Query("SELECT|people|age,name|")\n+\tif err != nil {\n+\t\tt.Fatal(err)\n+\t}\n+\n+\twant, fail := 2, errors.New("fail")\n+\tr := rows.rowsi.(*rowsCursor)\n+\tr.errPos, r.err = want, fail\n+\n+\tgot := 0\n+\tfor rows.Next() {\n+\t\tgot++\n+\t}\n+\tif got != want {\n+\t\tt.Errorf("got %d rows, want %d", got, want)\n+\t}\n+\tif err := rows.Err(); err != fail {\n+\t\tt.Errorf("got error %v, want %v", err, fail)\n+\t}\n+\tif !r.closed {\n+\t\tt.Errorf("r.closed is false, want true")\n+\t}\n+}\n+\n func TestStmtCloseOrder(t *testing.T) {\n \tdb := newTestDB(t, "people")\n \tdefer closeDB(t, db)\n```
## コアとなるコードの解説
### `src/pkg/database/sql/sql.go` の変更点
* **`Rows.Next()` メソッド**:
* 以前は `if rs.lasterr != nil { return false }` というチェックがありましたが、これは削除されました。これにより、`lasterr` が既に設定されている場合でも、`Next()` は次の行の読み込みを試みるか、またはエラー処理に進むようになります。
* 最も重要な変更は、`rs.lasterr = rs.rowsi.Next(rs.lastcols)` の呼び出し後です。
* 変更前: `if rs.lasterr == io.EOF { rs.Close() }`
* 変更後: `if rs.lasterr != nil { rs.Close(); return false }`
この変更により、`io.EOF` だけでなく、`rs.rowsi.Next()` が返す**あらゆる非nilエラー**に対して `rs.Close()` が呼び出されるようになりました。これにより、リソースリークの可能性が大幅に減少します。エラーが発生した場合は、すぐに `false` を返してイテレーションを終了させます。
* 最終的な戻り値のロジックも変更されました。
* 変更前: `return rs.lasterr == nil`
* 変更後: `return true` (エラーが発生しない限り)
これは、エラーが発生した場合は既に `false` を返して関数を抜けるため、エラーがなければ常に `true` を返すというシンプルなロジックになりました。
* **`Rows.lasterr` フィールドのコメント**: `lasterr error` のコメントが `lasterr error // non-nil only if closed is true` に変更されました。これは、`Next()` がエラー時に `Close()` を呼び出すようになったため、`Rows` が閉じられた状態でない限り `lasterr` は `nil` であるべきという新しい不変条件を示唆しています。
* **`Rows.Err()` メソッドのコメント**: `// Err may be called after an explicit or implicit Close.` というコメントが追加されました。これは、`Next()` が暗黙的に `Close()` を呼び出すようになったため、`Close()` 後でも `Err()` を呼び出してエラーを確認できることを明確にしています。
* **`Rows.Scan()` メソッド**:
* `if rs.lasterr != nil { return rs.lasterr }` というチェックが削除されました。これは、`Next()` がエラー時に `Close()` を呼び出すため、`Scan()` が呼び出される時点で `lasterr` が `nil` でない場合は、既に `Rows` が閉じられているはずだからです。
* `return errors.New("sql: Rows closed")` が `return errors.New("sql: Rows are closed")` に変更されました。これはメッセージの微調整です。
* **`Rows.Close()` メソッドのコメント**: `Next()` が `false` を返した場合に `Rows` が自動的に閉じられること、そして `Err()` の結果に影響を与えないことが明記されました。
### `src/pkg/database/sql/fakedb_test.go` の変更点
このファイルは、`database/sql` パッケージのテストで使用されるモックデータベースドライバの実装です。
* `rowsCursor` 構造体に `errPos int` と `err error` フィールドが追加されました。
* `errPos`: `Next()` メソッドがこの位置(行インデックス)に達したときに、意図的にエラーを発生させるためのカウンターです。
* `err`: `errPos` に達したときに返すエラーオブジェクトです。
* `rowsCursor.Next()` メソッドに `if rc.pos == rc.errPos { return rc.err }` というロジックが追加されました。これにより、テストケースで特定の行でエラーをシミュレートし、その後の `Rows.Close()` の暗黙的な呼び出しを検証できるようになりました。
### `src/pkg/database/sql/sql_test.go` の変更点
* `TestRowsImplicitClose` という新しいテスト関数が追加されました。
* このテストは、`fakedb_test.go` で追加された `errPos` と `err` を利用して、`Rows.Next()` が `io.EOF` 以外のエラーを返した場合に `Rows.Close()` が暗黙的に呼び出されることを検証します。
* 具体的には、`rowsCursor` の `errPos` を設定し、`Next()` ループが途中でエラーによって終了することを確認します。
* ループ終了後、`rows.Err()` が期待するエラーを返すこと、そして `rowsCursor` の `closed` フラグが `true` になっていることをアサートしています。これにより、エラー発生時に `Rows` が正しく閉じられたことが確認できます。
これらの変更により、`database/sql` パッケージの `Rows` オブジェクトは、より堅牢になり、開発者が `Close()` の呼び出しを忘れた場合でも、リソースリークのリスクが低減されるようになりました。
## 関連リンク
* [https://golang.org/cl/12677050](https://golang.org/cl/12677050)
## 参考にした情報源リンク
* Go言語 `database/sql` パッケージのドキュメント
* Go言語の `io.EOF` に関する一般的な情報
* Go言語の `defer` ステートメントに関する情報
* Go言語のテストに関する情報
* [Go issue 6005: database/sql: Rows.Next should implicitly call Rows.Close on error](https://github.com/golang/go/issues/6005) (このコミットに関連する可能性のあるGoのIssueトラッカー)
* このコミットは、`Rows.Next()` が `io.EOF` 以外のエラーで `false` を返した場合に `Rows.Close()` を暗黙的に呼び出すべきだという議論から生まれたものです。
* [Go issue 6006: database/sql: Rows.Scan should not return lasterr if Rows is closed](https://github.com/golang/go/issues/6006) (このコミットに関連する可能性のあるGoのIssueトラッカー)
* `Rows.Scan()` の `lasterr` チェック削除に関連する議論。
* [Go issue 6007: database/sql: Rows.Err should be callable after implicit Close](https://github.com/golang/go/issues/6007) (このコミットに関連する可能性のあるGoのIssueトラッカー)
* `Rows.Err()` のコメント変更に関連する議論。
* [Go issue 6008: database/sql: Rows.Next should not return false if lasterr is set](https://github.com/golang/go/issues/6008) (このコミットに関連する可能性のあるGoのIssueトラッカー)
* `Rows.Next()` の `lasterr` チェック削除に関連する議論。
* [Go issue 6009: database/sql: Rows.Close should be idempotent](https://github.com/golang/go/issues/6009) (このコミットに関連する可能性のあるGoのIssueトラッカー)
* `Rows.Close()` の冪等性に関する議論。
これらのIssueは、このコミットが解決しようとした問題や、その変更によって影響を受ける他の側面について、より深い洞察を提供します。