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

このコミットは、Go言語の database/sql パッケージにおける接続リークと潜在的なデッドロックの問題を修正するものです。具体的には、以前の変更 (CL 10726044) によって導入された競合状態が原因で、特定の状況下でデータベース接続がリークし、SetMaxOpenConns が使用されている場合にはアプリケーションが最終的にデッドロックに陥るか、そうでなければオープンな接続数が無制限に増加し続けるという問題に対処しています。

コミット

commit 37db8804691f0a5e618cbc041909895c9709263c
Author: Alberto García Hierro <alberto@garciahierro.com>
Date:   Wed Oct 16 09:22:57 2013 -0700

    database/sql: Fix connection leak and potential deadlock
    
    CL 10726044 introduced a race condition which causes connections
    to be leaked under certain circumstances. If SetMaxOpenConns is
    used, the application eventually deadlocks. Otherwise, the number
    of open connections just keep growing indefinitely.
    
    Fixes #6593
    
    R=golang-dev, bradfitz, tad.glines, bketelsen
    CC=golang-dev
    https://golang.org/cl/14611045

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

https://github.com/golang/go/commit/37db8804691f0a5e618cbc041909895c9709263c

元コミット内容

database/sql: Fix connection leak and potential deadlock

このコミットは、database/sql パッケージにおける接続リークと潜在的なデッドロックの問題を修正します。以前の変更 (CL 10726044) によって導入された競合状態が原因で、特定の状況下で接続がリークしていました。SetMaxOpenConns が使用されている場合、アプリケーションは最終的にデッドロックに陥り、そうでない場合はオープンな接続数が無制限に増加し続けていました。このコミットは、Issue #6593 を修正します。

変更の背景

このコミットの背景には、Go言語の database/sql パッケージにおける重要なバグが存在していました。具体的には、CL 10726044という以前の変更が、特定の条件下でデータベース接続のリークを引き起こす競合状態を導入してしまったことが問題の発端です。

database/sql パッケージは、GoアプリケーションがSQLデータベースと対話するための汎用的なインターフェースを提供します。このパッケージは、接続プールを管理し、アプリケーションが効率的にデータベース接続を利用できるように設計されています。接続プールは、新しい接続を確立するコストを削減し、データベースへの負荷を軽減するために非常に重要です。

しかし、このバグにより、接続が適切にプールに戻されず、リークしてしまう状況が発生していました。このリークは、アプリケーションの動作に深刻な影響を与えます。

1. 接続数の無制限な増加: SetMaxOpenConns が設定されていない場合、リークした接続はガベージコレクションされず、オープンな接続数が時間とともに際限なく増加します。これにより、データベースサーバーのリソースが枯渇し、最終的にはデータベースへの接続が拒否されるなどの問題が発生します。
2. デッドロック: SetMaxOpenConns が設定されている場合、オープンできる接続の最大数が制限されます。接続がリークすると、利用可能な接続が減少し、新しい接続を必要とするリクエストが接続プールからの解放を待つことになります。しかし、リークした接続が解放されないため、最終的にすべての利用可能な接続が使い果たされ、新しい接続を待つすべてのゴルーチンがブロックされ、アプリケーション全体がデッドロックに陥ります。これは、特に高負荷なシステムにおいて、サービス停止につながる重大な問題です。

この問題は、GoのIssueトラッカーで #6593 として報告され、その修正が急務となっていました。このコミットは、この深刻な接続リークとデッドロックの問題を解決するために導入されました。

前提知識の解説

このコミットの変更内容を理解するためには、以下のGo言語の database/sql パッケージに関する前提知識が必要です。

database/sql パッケージの概要

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

接続プール (Connection Pool)

database/sql パッケージの最も重要な機能の一つが接続プールです。データベースへの接続はコストの高い操作（ネットワークの確立、認証など）であるため、アプリケーションがリクエストごとに新しい接続を開閉するのは非効率的です。接続プールは、確立されたデータベース接続のセットを維持し、必要に応じてそれらを再利用することで、このオーバーヘッドを削減します。

• DB オブジェクト: sql.Open 関数によって返される *sql.DB オブジェクトは、データベースへの抽象的なハンドルであり、接続プールを管理します。このオブジェクトは、アプリケーションのライフサイクル全体で一度だけ作成され、共有されるべきです。
• SetMaxOpenConns(n int): このメソッドは、同時にオープンできるデータベース接続の最大数を設定します。n が0の場合、オープン接続数に制限はありません。デフォルトは0です。この設定は、データベースサーバーへの負荷を制御し、リソースの枯渇を防ぐために重要です。
• SetMaxIdleConns(n int): このメソッドは、アイドル状態（使用されていない）で接続プールに保持できる接続の最大数を設定します。n が0の場合、アイドル接続は保持されません。デフォルトは2です。アイドル接続は、新しいリクエストが来たときにすぐに利用できるため、パフォーマンスを向上させます。
• 接続の取得と解放: DB.Query(), DB.Exec() などのメソッドが呼び出されると、接続プールから接続が取得されます。クエリの実行が完了し、Rows オブジェクトが Close() されるか、エラーが発生した場合に、接続はプールに返却されます。

競合状態 (Race Condition)

競合状態とは、複数のゴルーチンが共有リソースに同時にアクセスし、そのアクセス順序によってプログラムの最終結果が非決定的に変わってしまう状況を指します。データベース接続プールのような共有リソースを扱うシステムでは、競合状態は接続のリーク、デッドロック、データの破損など、様々な問題を引き起こす可能性があります。

このコミットで修正される問題は、まさに database/sql パッケージ内部の接続管理ロジックにおける競合状態が原因でした。具体的には、新しい接続を開くプロセスにおいて、接続がプールに追加されるべきか、それともすぐに閉じられるべきかという判断が、複数のゴルーチンによって同時に行われる際に、誤ったパスが選択される可能性があったと考えられます。

putConnDBLocked 関数

database/sql パッケージの内部関数である putConnDBLocked は、接続をプールに戻す、または不要な接続を閉じる役割を担っています。この関数は、DBオブジェクトのロックを保持した状態で呼び出されることが想定されており、接続プールの状態を安全に更新します。

addDepLocked 関数

addDepLocked 関数は、database/sql パッケージ内部で、接続とそれに関連するリソース（例えば、Rows オブジェクト）との間の依存関係を追跡するために使用されます。これにより、Rows オブジェクトが閉じられるまで、基になる接続が閉じられないように保証されます。

これらの前提知識を理解することで、コミットがどのようにして接続リークとデッドロックの問題を解決しているのか、より深く把握することができます。

技術的詳細

このコミットは、database/sql パッケージにおける接続リークとデッドロックの問題を解決するために、主に DB.openNewConnection() メソッド内のロジックを変更しています。問題の根源は、新しい接続が確立された際に、その接続を接続プールに追加するべきか、それともすぐに閉じるべきかという判断が、競合状態によって誤って行われる可能性があった点にあります。

以前の実装では、openNewConnection 関数内で新しい接続 (dc) が作成された後、無条件に db.addDepLocked(dc, dc) と db.numOpen++ が実行され、その後 db.putConnDBLocked(dc, err) が呼び出されていました。

db.putConnDBLocked(dc, err) は、接続をプールに戻すか、またはエラーが発生した場合やプールが満杯の場合に接続を閉じる役割を担います。しかし、この関数が false を返す（つまり、接続がプールに追加されず、すぐに閉じられた）場合でも、db.addDepLocked と db.numOpen++ は既に実行されてしまっていました。

これが問題を引き起こしていました。

1. 接続リーク: putConnDBLocked が接続をプールに追加せずに閉じたにもかかわらず、db.numOpen がインクリメントされたままであるため、db.numOpen の値が実際のオープン接続数と一致しなくなります。これにより、SetMaxOpenConns の制限が正しく機能しなくなり、見かけ上オープンな接続数が上限に達していないにもかかわらず、実際には接続がリークしている状態が発生します。
2. デッドロック: SetMaxOpenConns が設定されている場合、db.numOpen が誤って増加したままだと、新しい接続を要求するゴルーチンが、実際には利用可能な接続があるにもかかわらず、db.numOpen が上限に達していると判断してブロックされ続けます。リークした接続が解放されないため、最終的にすべての接続が使い果たされ、アプリケーションがデッドロックに陥ります。

このコミットでは、このロジックの順序を修正し、putConnDBLocked の戻り値に基づいて db.addDepLocked と db.numOpen++ の実行を条件付きにすることで、この競合状態を解消しています。

修正のポイント:

• db.putConnDBLocked(dc, err) の呼び出しを先行させ、その戻り値を確認します。
• putConnDBLocked が true を返した場合（つまり、接続がプールに追加された場合）にのみ、db.addDepLocked(dc, dc) と db.numOpen++ を実行します。
• putConnDBLocked が false を返した場合（つまり、接続がプールに追加されず、すぐに閉じられた場合）は、ci.Close() を呼び出して、基になる接続を明示的に閉じます。これにより、不要な接続が確実に閉じられ、リソースが解放されます。

この変更により、db.numOpen は常に実際にオープンしている接続の数を正確に反映するようになり、接続リークとデッドロックの問題が解決されます。

また、この修正を検証するために、src/pkg/database/sql/fakedb_test.go に waitCh と waitingCh というチャネルが追加され、src/pkg/database/sql/sql_test.go に TestConnectionLeak という新しいテストケースが追加されています。このテストは、SetMaxOpenConns を設定した状態で、意図的に接続をブロックし、その後解放することで、接続リークが発生しないことを確認します。fakeDriver の Open メソッドにチャネルを介した同期メカニズムを導入することで、テストが接続のライフサイクルをより細かく制御できるようになっています。

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

src/pkg/database/sql/sql.go

--- a/src/pkg/database/sql/sql.go
+++ b/src/pkg/database/sql/sql.go
@@ -593,9 +593,12 @@ func (db *DB) openNewConnection() {
 		db: db,
 		ci: ci,
 	}
-	db.addDepLocked(dc, dc)
-	db.numOpen++
-	db.putConnDBLocked(dc, err)
+	if db.putConnDBLocked(dc, err) {
+		db.addDepLocked(dc, dc)
+		db.numOpen++
+	} else {
+		ci.Close()
+	}
 }
 
 // connRequest represents one request for a new connection

src/pkg/database/sql/fakedb_test.go

--- a/src/pkg/database/sql/fakedb_test.go
+++ b/src/pkg/database/sql/fakedb_test.go
@@ -38,6 +38,8 @@ type fakeDriver struct {
 	mu         sync.Mutex // guards 3 following fields
 	openCount  int        // conn opens
 	closeCount int        // conn closes
+	waitCh     chan struct{}
+	waitingCh  chan struct{}
 	dbs        map[string]*fakeDB
 }
 
@@ -146,6 +148,10 @@ func (d *fakeDriver) Open(dsn string) (driver.Conn, error) {
 	if len(parts) >= 2 && parts[1] == "badConn" {
 		conn.bad = true
 	}
+	if d.waitCh != nil {
+		d.waitingCh <- struct{}{}
+		<-d.waitCh
+	}
 	return conn, nil
 }
 

src/pkg/database/sql/sql_test.go

--- a/src/pkg/database/sql/sql_test.go
+++ b/src/pkg/database/sql/sql_test.go
@@ -1677,6 +1677,57 @@ func TestConcurrency(t *testing.T) {
 	doConcurrentTest(t, new(concurrentRandomTest))
 }
 
+func TestConnectionLeak(t *testing.T) {
+	db := newTestDB(t, "people")
+	defer closeDB(t, db)
+	// Start by opening defaultMaxIdleConns
+	rows := make([]*Rows, defaultMaxIdleConns)
+	// We need to SetMaxOpenConns > MaxIdleConns, so the DB can open
+	// a new connection and we can fill the idle queue with the released
+	// connections.
+	db.SetMaxOpenConns(len(rows) + 1)
+	for ii := range rows {
+		r, err := db.Query("SELECT|people|name|")
+		if err != nil {
+			t.Fatal(err)
+		}
+		r.Next()
+		if err := r.Err(); err != nil {
+			t.Fatal(err)
+		}
+		rows[ii] = r
+	}
+	// Now we have defaultMaxIdleConns busy connections. Open
+	// a new one, but wait until the busy connections are released
+	// before returning control to DB.
+	drv := db.driver.(*fakeDriver)
+	drv.waitCh = make(chan struct{}, 1)
+	drv.waitingCh = make(chan struct{}, 1)
+	var wg sync.WaitGroup
+	wg.Add(1)
+	go func() {
+		r, err := db.Query("SELECT|people|name|")
+		if err != nil {
+			t.Fatal(err)
+		}
+		r.Close()
+		wg.Done()
+	}()
+	// Wait until the goroutine we've just created has started waiting.
+	<-drv.waitingCh
+	// Now close the busy connections. This provides a connection for
+	// the blocked goroutine and then fills up the idle queue.
+	for _, v := range rows {
+		v.Close()
+	}
+	// At this point we give the new connection to DB. This connection is
+	// now useless, since the idle queue is full and there are no pending
+	// requests. DB should deal with this situation without leaking the
+	// connection.
+	drv.waitCh <- struct{}{}
+	wg.Wait()
+}
+
 func BenchmarkConcurrentDBExec(b *testing.B) {
 	b.ReportAllocs()
 	ct := new(concurrentDBExecTest)

コアとなるコードの解説

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

DB.openNewConnection() 関数は、新しいデータベース接続を確立し、それを接続プールに適切に組み込む役割を担っています。このコミットの核心的な変更は、この関数内のロジックの順序と条件分岐にあります。

変更前:

func (db *DB) openNewConnection() {
	// ... (接続の確立と初期化) ...
	db.addDepLocked(dc, dc)
	db.numOpen++
	db.putConnDBLocked(dc, err)
}

変更前は、新しい接続 dc が作成されると、まず db.addDepLocked(dc, dc) と db.numOpen++ が無条件に実行されていました。db.addDepLocked は接続の依存関係を追跡し、db.numOpen は現在オープンしている接続の数をインクリメントします。その後、db.putConnDBLocked(dc, err) が呼び出され、この関数が接続をプールに戻すか、またはエラーが発生した場合やプールが満杯の場合に接続を閉じるかを決定していました。

問題は、db.putConnDBLocked が接続をプールに追加せずに閉じることを決定した場合（例えば、プールが既に SetMaxIdleConns で設定されたアイドル接続数で満杯である場合）でも、db.numOpen は既にインクリメントされてしまっていた点です。これにより、db.numOpen が実際のオープン接続数よりも大きい値を示し、SetMaxOpenConns の制限が正しく機能しなくなり、接続リークやデッドロックの原因となっていました。

変更後:

func (db *DB) openNewConnection() {
	// ... (接続の確立と初期化) ...
	if db.putConnDBLocked(dc, err) {
		db.addDepLocked(dc, dc)
		db.numOpen++
	} else {
		ci.Close()
	}
}

変更後では、db.putConnDBLocked(dc, err) の呼び出しが先行し、その戻り値がチェックされるようになりました。

• db.putConnDBLocked(dc, err) が true を返した場合: これは、接続が正常にプールに追加されたことを意味します。この場合にのみ、db.addDepLocked(dc, dc) と db.numOpen++ が実行されます。これにより、db.numOpen は実際にプールに追加された（つまり、オープン状態にある）接続の数を正確に反映するようになります。
• db.putConnDBLocked(dc, err) が false を返した場合: これは、接続がプールに追加されず、すぐに閉じられたことを意味します（例えば、エラーが発生したか、アイドル接続プールが満杯で新しい接続が不要と判断された場合）。この場合、else ブロックが実行され、ci.Close() が呼び出されて、基になるデータベース接続が明示的に閉じられます。これにより、不要な接続が確実に解放され、リソースリークが防止されます。

この変更により、db.numOpen の値と実際のオープン接続数の間に不整合が生じる競合状態が解消され、SetMaxOpenConns が正しく機能するようになり、接続リークとデッドロックの問題が解決されました。

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

このファイルは、database/sql パッケージのテストで使用される偽のデータベースドライバ (fakeDriver) を定義しています。テストケースで接続のライフサイクルをより細かく制御できるように、以下のフィールドが fakeDriver 構造体に追加されました。

• waitCh chan struct{}: テストゴルーチンが偽のドライバの Open メソッドの実行を一時停止させるために使用されるチャネルです。
• waitingCh chan struct{}: 偽のドライバの Open メソッドが一時停止状態に入ったことをテストゴルーチンに通知するために使用されるチャネルです。

fakeDriver.Open() メソッド内では、d.waitCh が設定されている場合に、d.waitingCh <- struct{}{} で一時停止を通知し、<-d.waitCh でテストゴルーチンからの再開シグナルを待ちます。これにより、テスト中に特定のタイミングで接続のオープン処理をブロックし、競合状態をシミュレートすることが可能になります。

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

TestConnectionLeak という新しいテストケースが追加されました。このテストは、修正が正しく機能していることを検証するために設計されています。

テストの主な手順は以下の通りです。

1. defaultMaxIdleConns の数の接続をオープンし、それらをビジー状態（Rows オブジェクトが閉じられていない状態）に保ちます。これにより、アイドル接続プールが満杯になります。
2. SetMaxOpenConns を defaultMaxIdleConns + 1 に設定します。これにより、新しい接続を1つだけオープンできる余地ができます。
3. 新しいゴルーチンで db.Query() を呼び出し、新しい接続を要求します。このリクエストは、fakeDriver の Open メソッドによって一時的にブロックされます（drv.waitCh を使用）。
4. メインゴルーチンは、新しいゴルーチンがブロック状態に入ったことを drv.waitingCh を介して確認します。
5. メインゴルーチンは、最初にオープンしたすべてのビジー接続を閉じます。これにより、これらの接続がアイドル接続プールに戻され、プールが満杯になります。
6. メインゴルーチンは、drv.waitCh <- struct{}{} を介して、ブロックされていた新しいゴルーチンに処理を再開するシグナルを送ります。この時点で、新しい接続は不要と判断され、putConnDBLocked によって閉じられるべきです。
7. 最後に、wg.Wait() を使用して、新しいゴルーチンが完了するのを待ちます。

このテストの目的は、SetMaxOpenConns の制限があり、アイドル接続プールが満杯の状況で、新しい接続が不要と判断された場合に、その接続がリークせずに適切に閉じられることを確認することです。このテストが成功することで、openNewConnection の修正が接続リークとデッドロックの問題を効果的に解決していることが保証されます。

関連リンク

• Go GitHub リポジトリ: https://github.com/golang/go
• Go Issue #6593: https://github.com/golang/go/issues/6593 (このコミットが修正した問題のトラッキング)
• Go CL 14611045: https://golang.org/cl/14611045 (このコミットのコードレビューページ)

参考にした情報源リンク

• Go database/sql パッケージドキュメント: https://pkg.go.dev/database/sql
• Go database/sql/driver パッケージドキュメント: https://pkg.go.dev/database/sql/driver
• Go言語におけるデータベース接続の管理 (一般的な解説): https://go.dev/doc/database/ (Go公式ドキュメントのデータベースに関するセクション)
• Go言語の競合状態に関する一般的な情報 (例: Go Concurrency Patterns: Pipelines and Cancellation): https://go.dev/blog/pipelines (直接的な参照ではないが、競合状態の理解に役立つ)
• Go言語のチャネルに関する一般的な情報: https://go.dev/tour/concurrency/2 (テストコードの理解に役立つ)