Keyboard shortcuts

Press ← or → to navigate between chapters

Press S or / to search in the book

Press ? to show this help

Press Esc to hide this help

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

このコミットは、Go言語の標準ライブラリであるdatabase/sqlパッケージにおいて、ステートメント（Stmt）および行セット（Rows）のリソースリークを防ぐための修正を導入しています。特に、テスト用のfakedbドライバーに未クローズのStmtを検出するメカニズムを追加し、それに基づいて既存のコードベースにおけるリソース解放の漏れを修正しています。また、Tx.Queryの挙動をDB.Queryに合わせるための調整も含まれています。

コミット

commit c3954dd5da820306dff6bbd73a7801d9739b038f
Author: Gwenael Treguier <gwenn.kahz@gmail.com>
Date:   Sat Mar 10 15:21:44 2012 -0800

    database/sql: ensure Stmts are correctly closed.
    
    To make sure that there is no resource leak,
    I suggest to fix the 'fakedb' driver such as it fails when any
    Stmt is not closed.
    First, add a check in fakeConn.Close().
    Then, fix all missing Stmt.Close()/Rows.Close().
    I am not sure that the strategy choose in fakeConn.Prepare/prepare* is ok.
    The weak point in this patch is the change in Tx.Query:
      - Tests pass without this change,
      - I found it by manually analyzing the code,
      - I just try to make Tx.Query look like DB.Query.
    
    R=golang-dev, bradfitz
    CC=golang-dev
    https://golang.org/cl/5759050

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

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

元コミット内容

database/sql: ensure Stmts are correctly closed.

To make sure that there is no resource leak,
I suggest to fix the 'fakedb' driver such as it fails when any
Stmt is not closed.
First, add a check in fakeConn.Close().
Then, fix all missing Stmt.Close()/Rows.Close().
I am not sure that the strategy choose in fakeConn.Prepare/prepare* is ok.
The weak point in this patch is the change in Tx.Query:
  - Tests pass without this change,
  - I found it by manually analyzing the code,
  - I just try to make Tx.Query look like DB.Query.

R=golang-dev, bradfitz
CC=golang-dev
https://golang.org/cl/5759050

変更の背景

このコミットの主な背景は、database/sqlパッケージにおけるリソースリークの可能性を排除することです。データベース操作において、プリペアドステートメント（Stmt）やクエリ結果セット（Rows）は、使用後に適切にクローズされないと、データベース接続やメモリなどのシステムリソースを消費し続け、アプリケーションのパフォーマンス低下や最終的なリソース枯渇を引き起こす可能性があります。

特に、このコミットでは以下の点が問題視されていました。

  1. テスト環境でのリーク検出の困難さ: 実際のデータベース接続ではリソースリークが発生しても、テスト環境（特にfakedbのようなモックドライバー）ではその問題が顕在化しにくいという課題がありました。そのため、fakedbにリーク検出機能を追加することで、開発段階で問題を早期に発見できるようにする必要がありました。
  2. 既存コードにおけるリソース解放の漏れ: fakedbにリーク検出機能を追加した結果、既存のテストコードやdatabase/sqlパッケージ内部のコードで、StmtやRowsが適切にクローズされていない箇所が発見されました。これらの箇所を修正し、リソースが確実に解放されるようにする必要がありました。
  3. Tx.Queryの挙動の一貫性: Tx.Query（トランザクション内のクエリ）の挙動が、DB.Query（データベース接続全体のクエリ）と異なり、エラー発生時にStmtがクローズされないという不整合がありました。これにより、エラーパスでのリソースリークの可能性が生じていたため、両者の挙動を統一し、堅牢性を高める必要がありました。

これらの背景から、リソース管理の厳密化とコードの堅牢性向上を目的とした修正が導入されました。

前提知識の解説

このコミットを理解するためには、以下のGo言語のdatabase/sqlパッケージに関する知識が不可欠です。

database/sqlパッケージ

Go言語のdatabase/sqlパッケージは、SQLデータベースへの汎用的なインターフェースを提供します。これにより、特定のデータベースドライバーに依存しない形でデータベース操作を行うことができます。このパッケージは、データベースとの接続プール管理、プリペアドステートメントのキャッシュ、トランザクション管理などの機能を提供します。

driverインターフェース

database/sqlパッケージは、driverインターフェースを介して実際のデータベースドライバーと連携します。各データベース（例: MySQL, PostgreSQL, SQLite）は、このdriverインターフェースを実装することで、database/sqlパッケージから利用可能になります。

DB型

DB型は、データベースへの接続プールを表します。通常、アプリケーションの起動時に一度だけ作成され、複数のゴルーチンから安全に利用されます。

Tx型

Tx型は、データベーストランザクションを表します。DB.Begin()メソッドを呼び出すことで開始され、Commit()またはRollback()で終了します。トランザクション内で実行される操作は、すべてアトミックに処理されます。

Stmt型（プリペアドステートメント）

Stmt型は、プリペアドステートメントを表します。プリペアドステートメントは、SQLクエリを事前にデータベースに送信して準備しておくことで、繰り返し実行する際のパフォーマンスを向上させ、SQLインジェクション攻撃を防ぐのに役立ちます。 DB.Prepare()またはTx.Prepare()で作成され、Exec()やQuery()メソッドで実行されます。 重要: Stmtはデータベースリソースを消費するため、使用後は必ずStmt.Close()メソッドを呼び出して解放する必要があります。

Rows型（クエリ結果セット）

Rows型は、Query()メソッドの実行結果である行セットを表します。クエリ結果をイテレートして取得するために使用されます。 重要: Rowsもデータベースリソース（カーソルなど）を消費するため、すべての行を読み終えるか、エラーが発生した場合は、必ずRows.Close()メソッドを呼び出して解放する必要があります。通常はdefer rows.Close()のように記述されます。

リソースリーク

リソースリークとは、プログラムが獲得したシステムリソース（メモリ、ファイルハンドル、ネットワークソケット、データベース接続、プリペアドステートメントなど）を、使用後に適切に解放しないために、それらのリソースがシステム内に残り続けてしまう現象を指します。リソースリークが発生すると、システムのパフォーマンスが低下したり、最終的にはリソースが枯渇してアプリケーションがクラッシュしたりする原因となります。

fakedbドライバー

fakedbは、database/sqlパッケージのテストのために使用されるモック（偽の）データベースドライバーです。実際のデータベースに接続することなく、データベース操作のシミュレーションを行うことで、テストの実行速度を向上させ、外部依存性を排除します。このコミットでは、このfakedbにリソースリーク検出機能が追加されています。

技術的詳細

このコミットは、主に以下の3つの技術的側面に焦点を当てています。

  1. fakedbにおけるStmtリーク検出メカニズムの導入:

    • src/pkg/database/sql/fakedb_test.go内のfakeConn構造体に、stmtsMade（作成されたStmtの数）とstmtsClosed（クローズされたStmtの数）というカウンターが追加されました。
    • fakeConn.Prepareメソッドが呼び出されるたびにstmtsMadeがインクリメントされ、fakeStmt.Close()が呼び出されるたびにstmtsClosedがインクリメントされます。
    • fakeConn.Close()メソッドに、stmtsMade > stmtsClosedの場合にエラーを返すチェックが追加されました。これにより、接続がクローズされる際に未クローズのStmtが存在するとテストが失敗するようになり、リソースリークを検出できるようになりました。

  2. fakedbのPrepare関連メソッドにおけるエラーパスでのStmt.Close()の追加:

    • fakedb_test.go内のprepareSelect, prepareCreate, prepareInsertなどのprepare*ヘルパー関数や、fakeConn.Prepareメソッドにおいて、SQL構文エラーやその他の準備段階でのエラーが発生した場合に、作成途中のStmtインスタンスが適切にクローズされるようにstmt.Close()が追加されました。これにより、エラーパスでもリソースが確実に解放されるようになりました。

  3. Tx.QueryにおけるStmtのクローズロジックの修正:

    • src/pkg/database/sql/sql.go内のTx.Queryメソッドのロジックが変更されました。以前は、stmt.Query(args...)が成功した場合にのみrows.closeStmt = stmtが設定され、Rowsがクローズされる際にStmtもクローズされるようになっていました。
    • しかし、stmt.Query(args...)がエラーを返した場合、stmtはクローズされずに残ってしまう可能性がありました。
    • この修正では、stmt.Query(args...)がエラーを返した場合に、即座にstmt.Close()を呼び出すように変更されました。これにより、DB.Queryと同様に、Tx.Queryでもエラー発生時にStmtが確実にクローズされるようになり、一貫性と堅牢性が向上しました。

  4. sql_test.goにおけるStmt.Close()およびRows.Close()の追加:

    • src/pkg/database/sql/sql_test.go内の複数のテストケース（TestStatementQueryRow, TestExec, TestTxStmt, TestTxQuery, nullTestRunなど）において、StmtやRowsが使用された後にdefer stmt.Close()やdefer r.Close()が追加されました。これは、fakedbのリーク検出機能によって発見された既存のリーク箇所を修正するものです。これにより、テストコード自体がリソース管理のベストプラクティスに従うようになり、より信頼性の高いテストが可能になりました。

これらの変更により、database/sqlパッケージ全体のリソース管理が強化され、特にエラー発生時やテスト環境でのリソースリークが効果的に防止されるようになりました。

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

このコミットにおける主要なコード変更箇所は以下の通りです。

src/pkg/database/sql/fakedb_test.go

  1. fakeConn.Close()でのStmtリークチェックの追加:

    --- a/src/pkg/database/sql/fakedb_test.go
+++ b/src/pkg/database/sql/fakedb_test.go
@@ -214,6 +214,9 @@ func (c *fakeConn) Close() error {
 	if c.db == nil {
 		return errors.New("can't close fakeConn; already closed")
 	}
+	if c.stmtsMade > c.stmtsClosed {
+		return errors.New("can't close; dangling statement(s)")
+	}
 	c.db = nil
 	return nil
 }

  2. prepareSelectなどのエラーパスでのstmt.Close()の追加:

    --- a/src/pkg/database/sql/fakedb_test.go
+++ b/src/pkg/database/sql/fakedb_test.go
@@ -250,6 +253,7 @@ func errf(msg string, args ...interface{}) error {
 //  just a limitation for fakedb)
 func (c *fakeConn) prepareSelect(stmt *fakeStmt, parts []string) (driver.Stmt, error) {
 	if len(parts) != 3 {
+		stmt.Close()
 		return nil, errf("invalid SELECT syntax with %d parts; want 3", len(parts))
 	}
 	stmt.table = parts[0]
@@ -260,14 +264,17 @@ func (c *fakeConn) prepareSelect(stmt *fakeStmt, parts []string) (driver.Stmt, e
 		}
 		nameVal := strings.Split(colspec, "=")
 		if len(nameVal) != 2 {
+			stmt.Close()
 			return nil, errf("SELECT on table %q has invalid column spec of %q (index %d)", stmt.table, colspec, n)
 		}
 		column, value := nameVal[0], nameVal[1]
 		_, ok := c.db.columnType(stmt.table, column)
 		if !ok {
+			stmt.Close()
 			return nil, errf("SELECT on table %q references non-existent column %q", stmt.table, column)
 		}
 		if value != "?" {
+			stmt.Close()
 			return nil, errf("SELECT on table %q has pre-bound value for where column %q; need a question mark",
 				stmt.table, column)
 		}
@@ -280,12 +287,14 @@ func (c *fakeConn) prepareCreate(stmt *fakeStmt, parts []string) (driver.Stmt, e
 // parts are table|col=type,col2=type2
 func (c *fakeConn) prepareCreate(stmt *fakeStmt, parts []string) (driver.Stmt, error) {
 	if len(parts) != 2 {
+		stmt.Close()
 		return nil, errf("invalid CREATE syntax with %d parts; want 2", len(parts))
 	}
 	stmt.table = parts[0]
 	for n, colspec := range strings.Split(parts[1], ",") {
 		nameType := strings.Split(colspec, "=")
 		if len(nameType) != 2 {
+			stmt.Close()
 			return nil, errf("CREATE table %q has invalid column spec of %q (index %d)", stmt.table, colspec, n)
 		}
 		stmt.colName = append(stmt.colName, nameType[0])
@@ -297,17 +306,20 @@ func (c *fakeConn) prepareInsert(stmt *fakeStmt, parts []string) (driver.Stmt, e
 // parts are table|col=?,col2=val
 func (c *fakeConn) prepareInsert(stmt *fakeStmt, parts []string) (driver.Stmt, error) {
 	if len(parts) != 2 {
+		stmt.Close()
 		return nil, errf("invalid INSERT syntax with %d parts; want 2", len(parts))
 	}
 	stmt.table = parts[0]
 	for n, colspec := range strings.Split(parts[1], ",") {
 		nameVal := strings.Split(colspec, "=")
 		if len(nameVal) != 2 {
+			stmt.Close()
 			return nil, errf("INSERT table %q has invalid column spec of %q (index %d)", stmt.table, colspec, n)
 		}
 		column, value := nameVal[0], nameVal[1]
 		ctype, ok := c.db.columnType(stmt.table, column)
 		if !ok {
+			stmt.Close()
 			return nil, errf("INSERT table %q references non-existent column %q", stmt.table, column)
 		}
 		stmt.colName = append(stmt.colName, column)
@@ -323,10 +335,12 @@ func (c *fakeConn) prepareInsert(stmt *fakeStmt, parts []string) (driver.Stmt, e
 			case "int32":
 				i, err := strconv.Atoi(value)
 				if err != nil {
+					stmt.Close()
 					return nil, errf("invalid conversion to int32 from %q", value)
 				}
 				subsetVal = int64(i) // int64 is a subset type, but not int32
 			default:
+				stmt.Close()
 				return nil, errf("unsupported conversion for pre-bound parameter %q to type %q", value, ctype)
 			}
 			stmt.colValue = append(stmt.colValue, subsetVal)
@@ -362,6 +376,7 @@ func (c *fakeConn) Prepare(query string) (driver.Stmt, error) {
 	case "INSERT":
 		return c.prepareInsert(stmt, parts)
 	default:
+		stmt.Close()
 		return nil, errf("unsupported command type %q", cmd)
 	}
 	return stmt, nil

src/pkg/database/sql/sql.go

  1. Tx.Queryでのエラー時のStmtクローズロジックの修正: 
    --- a/src/pkg/database/sql/sql.go
+++ b/src/pkg/database/sql/sql.go
@@ -612,9 +612,11 @@ func (tx *Tx) Query(query string, args ...interface{}) (*Rows, error) {
 		return nil, err
 	}\n     	rows, err := stmt.Query(args...)\n    -\tif err == nil {\n    -\t\trows.closeStmt = stmt\n    +\tif err != nil {\n    +\t\tstmt.Close()\n    +\t\treturn nil, err\n     	}\n    +\trows.closeStmt = stmt\n     	return rows, err
 }\n     
@@ -1020,7 +1022,7 @@ func (r *Row) Scan(dest ...interface{}) error {
 	}\n     
 	// TODO(bradfitz): for now we need to defensively clone all\n    -\t// []byte that the driver returned (not permitting \n    +\t// []byte that the driver returned (not permitting\n     	// *RawBytes in Rows.Scan), since we're about to close\n     	// the Rows in our defer, when we return from this function.\n     	// the contract with the driver.Next(...) interface is that it

src/pkg/database/sql/sql_test.go

  1. 既存テストケースでのdefer stmt.Close()およびdefer r.Close()の追加: 
    --- a/src/pkg/database/sql/sql_test.go
+++ b/src/pkg/database/sql/sql_test.go
@@ -251,6 +251,7 @@ func TestStatementQueryRow(t *testing.T) {
 	if err != nil {
 		t.Fatalf("Prepare: %v", err)
 	}
+	defer stmt.Close()
 	var age int
 	for n, tt := range []struct {
 		name string
@@ -291,6 +292,7 @@ func TestExec(t *testing.T) {
 	if err != nil {
 		t.Errorf("Stmt, err = %v, %v", stmt, err)
 	}
+	defer stmt.Close()
 
 	type execTest struct {
 		args    []interface{}
@@ -332,11 +334,14 @@ func TestTxStmt(t *testing.T) {
 	if err != nil {
 		t.Fatalf("Stmt, err = %v, %v", stmt, err)
 	}
+	defer stmt.Close()
 	tx, err := db.Begin()
 	if err != nil {
 		t.Fatalf("Begin = %v", err)
 	}
-	_, err = tx.Stmt(stmt).Exec("Bobby", 7)
+	txs := tx.Stmt(stmt)
+	defer txs.Close()
+	_, err = txs.Exec("Bobby", 7)
 	if err != nil {
 		t.Fatalf("Exec = %v", err)
 	}
@@ -365,6 +370,7 @@ func TestTxQuery(t *testing.T) {
 	if err != nil {
 		t.Fatal(err)
 	}
+	defer r.Close()
 
 	if !r.Next() {
 		if r.Err() != nil {
@@ -561,6 +567,7 @@ func nullTestRun(t *testing.T, spec nullTestSpec) {
 	if err != nil {
 		t.Fatalf("prepare: %v", err)
 	}
+	defer stmt.Close()
 	if _, err := stmt.Exec(3, "chris", spec.rows[2].nullParam, spec.rows[2].notNullParam); err != nil {
 		t.Errorf("exec insert chris: %v", err)
 	}

コアとなるコードの解説

fakedb_test.goの変更

  • fakeConn.Close()でのリークチェック: fakeConnはテスト用の偽のデータベース接続を表します。この変更により、fakeConnがクローズされる際に、作成されたStmtの数（stmtsMade）とクローズされたStmtの数（stmtsClosed）を比較し、未クローズのStmtが存在すればエラーを発生させるようになりました。これは、fakedbを利用したテストにおいて、Stmtのリソースリークを自動的に検出するための重要なメカニズムです。これにより、開発者はテストを実行するだけで、Stmt.Close()の呼び出し忘れを特定できるようになります。

  • prepare*関数およびfakeConn.Prepareでのstmt.Close(): fakedbのPrepare関連の関数（prepareSelect, prepareCreate, prepareInsertなど）は、SQLクエリの解析や準備を行う際に、様々なエラーチェックを行います。これらのチェックでエラーが検出された場合、Stmtオブジェクトは完全に初期化されず、defer stmt.Close()のような通常のクリーンアップメカニズムが適用されない可能性があります。この変更は、エラーが発生した直後にstmt.Close()を明示的に呼び出すことで、部分的に作成されたStmtオブジェクトがリソースをリークするのを防ぎます。これは、エラーパスにおけるリソース管理の堅牢性を高めるための重要な修正です。

sql.goの変更

  • Tx.Queryでのエラー時のStmtクローズロジック: Tx.Queryメソッドは、トランザクション内でクエリを実行し、Rowsオブジェクトを返します。以前の実装では、stmt.Query(args...)が成功した場合にのみ、返されたRowsオブジェクトにstmtが関連付けられ、Rowsがクローズされる際にstmtもクローズされるようになっていました。しかし、stmt.Query(args...)自体がエラーを返した場合、stmtはRowsに関連付けられず、結果としてクローズされないまま残ってしまう可能性がありました。 この修正では、stmt.Query(args...)がエラーを返した場合に、即座にstmt.Close()を呼び出すように変更されました。これにより、Tx.Queryがエラーを返して終了する際にも、Stmtリソースが確実に解放されるようになり、DB.Queryの挙動との一貫性が保たれ、リソースリークのリスクが低減されました。

sql_test.goの変更

  • 既存テストケースでのdefer stmt.Close()およびdefer r.Close()の追加: これらの変更は、fakedbに導入されたリーク検出機能によって発見された、既存のテストコードにおけるリソース解放の漏れを修正するものです。deferキーワードを使用することで、関数が終了する際にstmt.Close()やr.Close()が確実に呼び出されるようになります。これはGo言語におけるリソース管理のベストプラクティスであり、テストコード自体がリソースリークの原因とならないようにするために重要です。これにより、テストの信頼性が向上し、将来的なリソースリークの混入を防ぐための良い例となります。

これらの変更は全体として、database/sqlパッケージのリソース管理をより厳密にし、特にエラー発生時やテスト環境でのリソースリークを防ぐことで、パッケージの堅牢性と信頼性を大幅に向上させています。

関連リンク

参考にした情報源リンク

  • Go言語のdatabase/sqlパッケージに関する一般的な情報源やチュートリアル。
  • Go言語におけるリソース管理、特にdeferステートメントの利用に関するベストプラクティス。
  • Go言語のテストにおけるモックとスタブの利用に関する情報。
  • Go言語のdatabase/sqlパッケージのソースコード（このコミットの変更点を含む）。
  • Go言語のコードレビュープロセスと、golang.org/cl（Gerrit Code Review）の利用方法。
  • Go言語のdatabase/sqlパッケージの設計思想に関する議論やドキュメント。
  • Go言語のdatabase/sqlパッケージにおけるトランザクション管理に関する情報。
  • Go言語におけるエラーハンドリングのパターン。
  • Go言語のdatabase/sqlパッケージの歴史的な変更ログや関連するIssue。
  • Go言語のdatabase/sqlパッケージのStmtとRowsのライフサイクルに関する詳細な解説。