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

このコミットは、Go言語の標準ライブラリ net/http パッケージにおける Transport のメモリリークを修正するものです。具体的には、アイドル状態のコネクションを管理するためのチャネル (idleConnCh) が適切にクリーンアップされず、メモリリークを引き起こす可能性があった問題に対処しています。

コミット

commit 46161cd0798c9d80af53abd65875459658f22f6e
Author: Brad Fitzpatrick <bradfitz@golang.org>
Date:   Fri Jun 28 12:57:54 2013 -0700

    net/http: fix memory leak in Transport
    
    Fixes #5794
    
    R=golang-dev, r
    CC=golang-dev
    https://golang.org/cl/10747044

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

https://github.com/golang/go/commit/46161cd0798c9d80af53abd65875459658f22f6e

元コミット内容

net/http: fix memory leak in Transport

Fixes #5794

R=golang-dev, r
CC=golang-dev
https://golang.org/cl/10747044

変更の背景

このコミットは、Goの net/http パッケージの Transport におけるメモリリークを修正するために行われました。コミットメッセージには Fixes #5794 と記載されていますが、現在のGitHubリポジトリではこのIssue番号に対応する具体的なIssueは見つかりませんでした。しかし、コミット内容から推測すると、HTTPクライアントがアイドル状態のコネクションを管理する際に、特定の条件下でコネクションチャネルが適切に閉じられず、メモリが解放されない問題が発生していたと考えられます。

特に、Transport が DisableKeepAlives を true に設定している場合や、コネクションがアイドル状態になった際に、そのコネクションを待機しているダイアラーが存在しない場合に、idleConnCh マップのエントリが残り続けることが問題でした。これにより、使用されなくなったチャネルがメモリ上に残り続け、アプリケーションのメモリ使用量が増加する可能性がありました。

前提知識の解説

このコミットを理解するためには、Goの net/http パッケージにおける以下の概念を理解しておく必要があります。

• net/http.Client: HTTPリクエストを送信するためのクライアント。通常、http.DefaultClient を使用するか、カスタムの http.Client を作成して使用します。
• net/http.Transport: http.Client の内部で使用され、実際のHTTPリクエストの送信、コネクションの確立、コネクションプーリング、Keep-Aliveの管理など、低レベルのネットワーク操作を担当します。
• Keep-Alive (持続的接続): HTTP/1.1で導入された機能で、複数のHTTPリクエスト/レスポンスを同じTCPコネクション上で送受信することを可能にします。これにより、コネクションの確立と切断のオーバーヘッドが削減され、パフォーマンスが向上します。
• コネクションプーリング: Transport は、再利用可能なアイドル状態のTCPコネクションをプールします。これにより、新しいリクエストが来た際に既存のコネクションを再利用でき、コネクション確立のコストを削減できます。
• idleConn: Transport 内部で管理される、アイドル状態のコネクションを格納するマップです。キーはホスト名など、値は *persistConn のスライスです。
• idleConnCh: Transport 内部で管理される、アイドル状態のコネクションを待機しているゴルーチンにコネクションを渡すためのチャネルのマップです。キーはホスト名など、値は chan *persistConn です。新しいコネクションが必要な際に、このチャネルから既存のアイドルコネクションを受け取ることができます。
• persistConn: 永続的な（Keep-Alive対応の）HTTPコネクションを表す内部構造体です。

メモリリークは、プログラムが不要になったメモリを解放せず、そのメモリを保持し続けることで発生します。Goではガベージコレクションがありますが、参照が残っているオブジェクトはガベージコレクションの対象とならないため、意図しない参照がメモリリークの原因となることがあります。

技術的詳細

このメモリリークは、Transport の putIdleConn メソッドと getIdleConnCh メソッドの相互作用によって引き起こされていました。

1. putIdleConn メソッド: このメソッドは、使用済みでアイドル状態になった persistConn をプールに戻す役割を担います。

   • 以前の実装では、t.idleConnCh[key] にコネクションを送信しようと試みていました。これは、そのコネクションタイプを待機しているダイアラーがいる場合に、そのダイアラーにコネクションを直接渡すためです。
   • しかし、select ステートメントの default ブロックに入った場合（つまり、待機しているダイアラーがいなかった場合）、t.idleConnCh[key] にチャネルが残ったままになる可能性がありました。特に、DisableKeepAlives が true の場合や、ダイアラーがコネクションを待つ前に別の方法でコネクションを取得した場合に、このチャネルが不要になってもマップから削除されない問題がありました。

2. getIdleConnCh メソッド: このメソッドは、特定のコネクションタイプに対するアイドルコネクションチャネルを返します。

   • 以前の実装では、t.DisableKeepAlives が true の場合でも、idleConnCh マップにエントリが作成される可能性がありました。DisableKeepAlives が true の場合、アイドルコネクションは使用されないため、関連するチャネルも不要になります。

修正のポイント:

• putIdleConn における idleConnCh のクリーンアップ:
  • putIdleConn メソッド内で、select ステートメントの default ブロックに入った際に、waitingDialer が nil でない（つまり、チャネルが存在していた）場合、delete(t.idleConnCh, key) を呼び出して、不要になったチャネルエントリをマップから明示的に削除するように変更されました。これにより、待機者がいなくなったチャネルがメモリに残り続けることを防ぎます。
• CloseIdleConnections における idleConnCh のリセット:
  • CloseIdleConnections メソッド内で、t.idleConn = nil と共に t.idleConnCh = nil が追加されました。これにより、アイドルコネクションが閉じられる際に、関連するチャネルマップも完全にリセットされ、残存するチャネルがなくなるようにします。
• getIdleConnCh における DisableKeepAlives の考慮:
  • getIdleConnCh メソッドの冒頭で、t.DisableKeepAlives が true の場合は即座に nil を返すように変更されました。これにより、Keep-Aliveが無効な場合に不要なチャネルが作成されたり、マップに追加されたりすることを防ぎます。

これらの変更により、Transport がアイドルコネクションを管理する際に、不要になったチャネルが idleConnCh マップに残り続けることがなくなり、メモリリークが解消されます。

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

変更は主に以下の3つのファイルで行われています。

1. src/pkg/net/http/export_test.go: テストのために IdleConnChMapSizeForTesting というヘルパー関数が追加されました。これは idleConnCh マップの現在のサイズを返すもので、メモリリークのテストに使用されます。
2. src/pkg/net/http/transport.go: Transport の主要なロジックが含まれるファイルで、メモリリークの修正が行われました。
3. src/pkg/net/http/transport_test.go: メモリリークが修正されたことを検証するための新しいテストケース TestIdleConnChannelLeak が追加されました。

src/pkg/net/http/transport.go の変更点

--- a/src/pkg/net/http/transport.go
+++ b/src/pkg/net/http/transport.go
@@ -217,6 +217,7 @@ func (t *Transport) CloseIdleConnections() {
 	t.idleMu.Lock()
 	m := t.idleConn
 	t.idleConn = nil
+	t.idleConnCh = nil
 	t.idleMu.Unlock()
 	if m == nil {
 		return
@@ -295,8 +296,10 @@ func (t *Transport) putIdleConn(pconn *persistConn) bool {
 		max = DefaultMaxIdleConnsPerHost
 	}
 	t.idleMu.Lock()
+
+	waitingDialer := t.idleConnCh[key]
 	select {
-	case t.idleConnCh[key] <- pconn:
+	case waitingDialer <- pconn:
 		// We're done with this pconn and somebody else is
 		// currently waiting for a conn of this type (they're
 		// actively dialing, but this conn is ready
@@ -305,6 +308,11 @@ func (t *Transport) putIdleConn(pconn *persistConn) bool {
 		t.idleMu.Unlock()
 		return true
 	default:
+		if waitingDialer != nil {
+			// They had populated this, but their dial won
+			// first, so we can clean up this map entry.
+			delete(t.idleConnCh, key)
+		}
 	}
 	if t.idleConn == nil {
 		t.idleConn = make(map[string][]*persistConn)
@@ -324,7 +332,13 @@ func (t *Transport) putIdleConn(pconn *persistConn) bool {
 	return true
 }
 
+// getIdleConnCh returns a channel to receive and return idle
+// persistent connection for the given connectMethod.
+// It may return nil, if persistent connections are not being used.
 func (t *Transport) getIdleConnCh(cm *connectMethod) chan *persistConn {
+	if t.DisableKeepAlives {
+		return nil
+	}
 	key := cm.key()
 	t.idleMu.Lock()
 	defer t.idleMu.Unlock()

src/pkg/net/http/transport_test.go の変更点

--- a/src/pkg/net/http/transport_test.go
+++ b/src/pkg/net/http/transport_test.go
@@ -1575,6 +1575,41 @@ func TestProxyFromEnvironment(t *testing.T) {
 	}
 }
 
+func TestIdleConnChannelLeak(t *testing.T) {
+	var mu sync.Mutex
+	var n int
+
+	ts := httptest.NewServer(HandlerFunc(func(w ResponseWriter, r *Request) {
+		mu.Lock()
+		n++
+		mu.Unlock()
+	}))
+	defer ts.Close()
+
+	tr := &Transport{
+		Dial: func(netw, addr string) (net.Conn, error) {
+			return net.Dial(netw, ts.Listener.Addr().String())
+		},
+	}
+	defer tr.CloseIdleConnections()
+
+	c := &Client{Transport: tr}
+
+	// First, without keep-alives.
+	for _, disableKeep := range []bool{true, false} {
+		tr.DisableKeepAlives = disableKeep
+		for i := 0; i < 5; i++ {
+			_, err := c.Get(fmt.Sprintf("http://foo-host-%d.tld/", i))
+			if err != nil {
+				t.Fatal(err)
+			}
+		}
+		if got := tr.IdleConnChMapSizeForTesting(); got != 0 {
+			t.Fatalf("ForDisableKeepAlives = %v, map size = %d; want 0", disableKeep, got)
+		}
+	}
+}
+
 // rgz is a gzip quine that uncompresses to itself.
 var rgz = []byte{
 	0x1f, 0x8b, 0x08, 0x08, 0x00, 0x00, 0x00, 0x00,

コアとなるコードの解説

transport.go の変更点

1. CloseIdleConnections() メソッド:

   	t.idleConn = nil
   	t.idleConnCh = nil // 追加
   

   CloseIdleConnections は、アイドル状態のコネクションをすべて閉じる際に呼び出されます。以前は t.idleConn (アイドルコネクションのマップ) のみを nil にしていましたが、この変更で t.idleConnCh (アイドルコネクションチャネルのマップ) も nil に設定されるようになりました。これにより、コネクションが閉じられた際に、関連するチャネルも確実に解放され、メモリリークを防ぎます。

2. putIdleConn() メソッド:

   	waitingDialer := t.idleConnCh[key] // 変更: 直接マップから取得
   	select {
   	case waitingDialer <- pconn: // 変更: waitingDialer を使用
   		// ...
   	default:
   		if waitingDialer != nil { // 追加
   			// They had populated this, but their dial won
   			// first, so we can clean up this map entry.
   			delete(t.idleConnCh, key) // 追加
   		}
   	}
   

   putIdleConn は、使用済みの persistConn をアイドルプールに戻す際に呼び出されます。

   • 以前は t.idleConnCh[key] <- pconn と直接マップの要素に送信しようとしていましたが、waitingDialer := t.idleConnCh[key] で一度変数に格納し、その変数に送信するように変更されました。これは、select の default ブロックで waitingDialer の状態をチェックするためです。
   • 最も重要な変更は default ブロック内です。select が default にフォールバックした場合（つまり、pconn を受け取る準備ができているダイアラーがいなかった場合）、waitingDialer が nil でない（つまり、チャネルが存在していた）ならば、delete(t.idleConnCh, key) を呼び出して、そのチャネルエントリを idleConnCh マップから削除します。これは、ダイアラーが既に別の方法でコネクションを取得したか、Keep-Aliveが無効になっているためにチャネルが不要になった場合に、不要なチャネルがマップに残り続けることを防ぐためのものです。

3. getIdleConnCh() メソッド:

   func (t *Transport) getIdleConnCh(cm *connectMethod) chan *persistConn {
   	if t.DisableKeepAlives { // 追加
   		return nil // 追加
   	}
   	// ...
   }
   

   getIdleConnCh は、アイドルコネクションを受け取るためのチャネルを返します。この変更により、Transport の DisableKeepAlives が true に設定されている場合（つまり、Keep-Aliveが無効な場合）は、チャネルを生成せずに即座に nil を返すようになりました。これにより、Keep-Aliveが不要な場合に、idleConnCh マップに不要なエントリが作成されることを防ぎます。

transport_test.go の変更点

TestIdleConnChannelLeak という新しいテストケースが追加されました。 このテストは、Transport の DisableKeepAlives を true と false の両方で試行し、複数のHTTPリクエストを送信します。 その後、tr.IdleConnChMapSizeForTesting() を呼び出して idleConnCh マップのサイズを確認します。期待される結果は 0 であり、これによりメモリリークが修正されたことを検証します。

関連リンク

• Go言語の net/http パッケージのドキュメント: https://pkg.go.dev/net/http
• Go言語の Transport 構造体に関するドキュメント: https://pkg.go.dev/net/http#Transport

参考にした情報源リンク

• Go言語の公式リポジトリ: https://github.com/golang/go
• Go言語のコードレビューシステム (Gerrit): https://go.dev/cl/10747044 (コミットメッセージに記載されているCLリンク)
• Go言語のIssueトラッカー (GitHub Issues): https://github.com/golang/go/issues (ただし、#5794は直接見つからず)
• Go言語におけるメモリリークに関する一般的な情報源 (例: Goのガベージコレクション、プロファイリングツールなど)