[インデックス 17212] ファイルの概要

このコミットは、Go言語の標準ライブラリである database/sql パッケージに、既知のバグを再現するためのテストケースを追加するものです。このテストは意図的に無効化されており、Rows.Close() メソッドが driver.ErrBadConn を返した場合に、内部的なステートが正しくクリーンアップされない問題（Issue 6081）を浮き彫りにすることを目的としています。

コミット

commit ca3ed9f3520de7998dcc009eca8d35eefec55412
Author: Brad Fitzpatrick <bradfitz@golang.org>
Date:   Tue Aug 13 14:56:40 2013 -0700

    database/sql: add a disabled broken test
    
    Update #6081
    
    R=golang-dev, gri
    CC=golang-dev
    https://golang.org/cl/12810043

GitHub上でのコミットページへのリンク

https://github.com/golang/go/commit/ca3ed9f3520de7998dcc009eca8d35eefec55412

元コミット内容

database/sql: add a disabled broken test

Update #6081

R=golang-dev, gri
CC=golang-dev
https://golang.org/cl/12810043

変更の背景

このコミットは、Go言語の database/sql パッケージにおける特定のバグ、具体的には Issue 6081 に関連しています。このIssueは、Rows.Close() メソッドが driver.ErrBadConn エラーを返した場合に、database/sql パッケージの内部状態が適切にクリーンアップされないという問題点を指摘していました。

通常、Rows.Close() は、データベースからの結果セットの読み取りを終了し、関連するリソース（例えば、データベース接続やステートメント）を解放するために呼び出されます。しかし、このクローズ処理中に driver.ErrBadConn のような致命的なエラーが発生した場合、database/sql パッケージは、そのエラーを適切に処理し、関連する内部ステート（特に stmt.css スライス）をクリアするべきです。Issue 6081は、このクリーンアップが不完全であるために、後続の操作で問題が発生する可能性を示唆していました。

このコミットの目的は、この特定のバグを再現するためのテストケースを導入することです。テストは意図的に t.Skip("known broken test") で無効化されており、これはテストが現在の実装では失敗すること、そしてその失敗が既知のバグによるものであることを示しています。このような「壊れたテスト」を追加する一般的なプラクティスは、バグの存在を明確にし、将来的にバグが修正された際にその修正を検証するための基盤を提供することです。

前提知識の解説

`database/sql` パッケージ

database/sql はGo言語の標準ライブラリの一部であり、GoプログラムからSQLデータベースにアクセスするための汎用的なインターフェースを提供します。このパッケージ自体は特定のデータベースドライバを含まず、データベース固有の操作は database/sql/driver インターフェースを実装する外部ドライバに委譲されます。

主要な概念:

DB: データベースへの接続プールを表します。
Stmt: プリペアドステートメントを表します。SQLインジェクションを防ぎ、クエリのパフォーマンスを向上させます。
Rows: クエリの結果セットを表します。Next() メソッドで次の行に進み、Scan() メソッドでデータをGoの変数に読み込みます。
Tx: トランザクションを表します。
driver.Driver: データベースドライバが実装すべきインターフェース。
driver.Conn: データベース接続が実装すべきインターフェース。
driver.Stmt: ドライバのプリペアドステートメントが実装すべきインターフェース。
driver.Rows: ドライバの結果セットが実装すべきインターフェース。
driver.ErrBadConn: ドライバが返す可能性のあるエラーの一つで、接続が不良状態であることを示します。このエラーが返された場合、database/sql パッケージは通常、その接続を接続プールから削除し、新しい接続を確立しようとします。

`Rows.Close()` メソッド

Rows.Close() メソッドは、結果セットのイテレーションを終了し、関連するデータベースリソースを解放するために呼び出されます。これは非常に重要で、リソースリークを防ぐために、結果セットの処理が完了したら常に呼び出す必要があります（通常は defer rows.Close() の形式で）。

`stmt.css` スライス

stmt.css は database/sql パッケージの内部的なフィールドで、Stmt オブジェクトに関連付けられた driver.Rows オブジェクトのリストを保持するために使用されます。Stmt.Query() が呼び出されるたびに、新しい driver.Rows オブジェクトが作成され、このスライスに追加されます。Rows.Close() が呼び出されると、対応する driver.Rows オブジェクトがこのスライスから削除されることが期待されます。このスライスが適切にクリーンアップされないと、メモリリークや、Stmt オブジェクトが不要なリソースへの参照を保持し続けるといった問題が発生する可能性があります。

`t.Skip()`

Goのテストフレームワーク (testing パッケージ) において、t.Skip() はテスト関数内で呼び出されると、そのテストをスキップします。これは、まだ修正されていないバグを再現するテストや、特定の環境でのみ実行されるべきテストなど、一時的に実行したくないテストに使用されます。

技術的詳細

このコミットは、database/sql パッケージの Rows 型に rowsCloseHook という新しいグローバル変数（関数型）を追加し、Rows.Close() メソッドの動作を変更しています。

具体的には、Rows.Close() メソッド内で、実際のドライバの rowsi.Close() メソッドが呼び出された直後に、rowsCloseHook が設定されていればそれを呼び出すように変更されています。このフック関数は、現在の Rows オブジェクトと、rowsi.Close() から返されたエラーへのポインタを受け取ります。

テストケース TestIssue6081 では、この rowsCloseHook を利用して、Rows.Close() が driver.ErrBadConn を返す状況をシミュレートしています。テストは以下の手順で実行されます。

newTestDB でテスト用のデータベース接続を作成します。
db.Prepare でプリペアドステートメントを作成します。
rowsCloseHook を設定し、Rows.Close() が呼び出された際に *err = driver.ErrBadConn となるようにします。
ループ内で10回 stmt.Query() を呼び出し、すぐに rows.Close() を呼び出します。これにより、driver.ErrBadConn が繰り返し発生する状況を作り出します。
ループ終了後、stmt.css スライスの長さが1以下であることを検証します。これは、Rows.Close() がエラーを返しても、stmt.css から driver.Rows オブジェクトが適切に削除されるべきであることを示しています。
stmt.Close() を呼び出した後、stmt.css スライスの長さが0であることを検証します。これは、ステートメントがクローズされた後、関連するすべてのリソースが解放されるべきであることを示しています。
最後に、データベースドライバの openCount と closeCount を検証し、接続の開閉が期待通りに行われていることを確認します。

このテストの核心は、Rows.Close() がエラーを返した場合でも、Stmt オブジェクトの内部状態（特に stmt.css スライス）が正しく管理され、リソースリークや不正な状態にならないことを確認することです。driver.ErrBadConn は、接続が再利用できないことを示すため、database/sql パッケージは、その接続に関連するすべてのリソースをクリーンアップし、新しい接続を確立する準備をする必要があります。このテストは、そのクリーンアップが stmt.css に対して適切に行われているかを検証します。

コアとなるコードの変更箇所

`src/pkg/database/sql/sql.go`

@@ -1372,6 +1372,8 @@ func (rs *Rows) Scan(dest ...interface{}) error {
 	return nil
 }
 
+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.
@@ -1381,6 +1383,9 @@ func (rs *Rows) Close() error {
 	}
 	rs.closed = true
 	err := rs.rowsi.Close()
+	if fn := rowsCloseHook; fn != nil {
+		fn(rs, &err)
+	}
 	if rs.closeStmt != nil {
 		rs.closeStmt.Close()
 	}

`src/pkg/database/sql/sql_test.go`

@@ -5,6 +5,7 @@
 package sql
 
 import (
+	"database/sql/driver"
 	"fmt"
 	"reflect"
 	"runtime"
@@ -1110,6 +1111,52 @@ func manyConcurrentQueries(t testOrBench) {
 	wg.Wait()
 }
 
+func TestIssue6081(t *testing.T) {
+	t.Skip("known broken test")
+	db := newTestDB(t, "people")
+	defer closeDB(t, db)
+
+	drv := db.driver.(*fakeDriver)
+	drv.mu.Lock()
+	opens0 := drv.openCount
+	closes0 := drv.closeCount
+	drv.mu.Unlock()
+
+	stmt, err := db.Prepare("SELECT|people|name|")
+	if err != nil {
+		t.Fatal(err)
+	}
+	rowsCloseHook = func(rows *Rows, err *error) {
+		*err = driver.ErrBadConn
+	}
+	defer func() { rowsCloseHook = nil }()
+	for i := 0; i < 10; i++ {
+		rows, err := stmt.Query()
+		if err != nil {
+			t.Fatal(err)
+		}
+		rows.Close()
+	}
+	if n := len(stmt.css); n > 1 {
+		t.Errorf("len(css slice) = %d; want <= 1", n)
+	}
+	stmt.Close()
+	if n := len(stmt.css); n != 0 {
+		t.Errorf("len(css slice) after Close = %d; want 0", n)
+	}
+
+	drv.mu.Lock()
+	opens := drv.openCount - opens0
+	closes := drv.closeCount - closes0
+	drv.mu.Unlock()
+	if opens < 9 {
+		t.Errorf("opens = %d; want >= 9", opens)
+	}
+	if closes < 9 {
+		t.Errorf("closes = %d; want >= 9", closes)
+	}
+}
+
 func TestConcurrency(t *testing.T) {
 	manyConcurrentQueries(t)
 }

コアとなるコードの解説

`src/pkg/database/sql/sql.go` の変更

var rowsCloseHook func(*Rows, *error) の追加: Rows.Close() メソッドの動作をテストからフックするためのグローバル変数 rowsCloseHook が追加されました。これは関数型であり、*Rows と *error を引数にとります。これにより、テストコードが Rows.Close() の内部で発生するエラーをシミュレートしたり、その動作を監視したりすることが可能になります。
Rows.Close() メソッド内のフック呼び出し: Rows.Close() メソッド内で、rs.rowsi.Close() が呼び出された直後に rowsCloseHook が設定されていれば呼び出されるようになりました。
```
if fn := rowsCloseHook; fn != nil {
    fn(rs, &err)
}
```
ここで &err を渡すことで、フック関数内で rs.rowsi.Close() から返されたエラーを変更できるようになっています。これは、テストが driver.ErrBadConn のような特定のエラーを強制的に発生させるために利用されます。

`src/pkg/database/sql/sql_test.go` の変更

import "database/sql/driver" の追加: driver.ErrBadConn を使用するために database/sql/driver パッケージがインポートされました。
TestIssue6081 関数の追加: このテスト関数は、Issue 6081で報告されたバグを再現するために設計されています。
- t.Skip("known broken test"): この行により、テストはデフォルトでスキップされます。これは、このテストが現在の実装では失敗することが既知であり、バグが修正されるまで通常のテストスイートの一部として実行されるべきではないことを示しています。
- rowsCloseHook = func(rows *Rows, err *error) { *err = driver.ErrBadConn }: この行がテストの核心です。rowsCloseHook グローバル変数に匿名関数を割り当てることで、Rows.Close() が呼び出された際に、そのエラー引数 err を driver.ErrBadConn に上書きするように設定しています。これにより、Rows.Close() が接続不良エラーを返す状況をシミュレートします。
- defer func() { rowsCloseHook = nil }(): テスト関数の終了時に rowsCloseHook を nil にリセットすることで、他のテストに影響を与えないようにしています。
- ループ内での Query() と Close() の呼び出し: for i := 0; i < 10; i++ ループ内で、stmt.Query() を呼び出して Rows オブジェクトを取得し、すぐに rows.Close() を呼び出しています。これにより、driver.ErrBadConn が繰り返し発生する状況を作り出し、stmt.css スライスのクリーンアップが正しく行われるかを検証します。
- len(stmt.css) の検証: if n := len(stmt.css); n > 1 { t.Errorf(...) } と if n := len(stmt.css); n != 0 { t.Errorf(...) } のアサーションは、Rows.Close() がエラーを返した場合でも、stmt.css スライスが適切に縮小され、最終的には stmt.Close() 後に空になることを確認します。これは、リソースリークや不正な内部状態を防ぐために重要です。
- openCount と closeCount の検証: fakeDriver の openCount と closeCount を確認することで、データベース接続の開閉が期待通りに行われているかを検証しています。これは、driver.ErrBadConn が発生した場合に、database/sql パッケージが新しい接続を確立しようとする動作を間接的に確認するものです。

このテストは、database/sql パッケージが Rows.Close() からのエラー、特に driver.ErrBadConn を適切に処理し、内部状態を健全に保つことの重要性を示しています。

参考にした情報源リンク

上記のGitHub IssueおよびGo言語の公式ドキュメント。
Go言語のソースコード（src/pkg/database/sql/sql.go および src/pkg/database/sql/sql_test.go）。
database/sql パッケージの一般的な使用方法と内部動作に関する知識。
Go言語のテストフレームワーク (testing パッケージ) に関する知識。
driver.ErrBadConn の意味と database/sql パッケージでのその処理に関する知識。

comemo