[インデックス 14677] ファイルの概要
このコミットは、Go言語の net/http
パッケージにおける Transport
のバグ修正に関するものです。具体的には、HTTPリクエストの ContentLength
が明示的に短く設定されている場合に発生するゴルーチンリークを修正し、それに対応するテストケースを追加しています。
コミット
commit 7c3577e48f629120604d232c7a3994cf40ae4cda
Author: Brad Fitzpatrick <bradfitz@golang.org>
Date: Mon Dec 17 12:01:00 2012 -0800
net/http: fix goroutine leak in error case
Fixes #4531
R=golang-dev, rsc
CC=golang-dev
https://golang.org/cl/6937069
---
src/pkg/net/http/transport.go | 1 +\
src/pkg/net/http/transport_test.go | 39 ++++++++++++++++++++++++++++++++++++++\n 2 files changed, 40 insertions(+)
diff --git a/src/pkg/net/http/transport.go b/src/pkg/net/http/transport.go
index 1dd5cc5308..d0505bf13f 100644
--- a/src/pkg/net/http/transport.go
+++ b/src/pkg/net/http/transport.go
@@ -742,6 +742,7 @@ WaitResponse:
case err := <-writeErrCh:
if err != nil {
re = responseAndError{nil, err}
+ pc.close()
break WaitResponse
}
case <-pconnDeadCh:
diff --git a/src/pkg/net/http/transport_test.go b/src/pkg/net/http/transport_test.go
index 4647d20fb3..c37ef13a41 100644
--- a/src/pkg/net/http/transport_test.go
+++ b/src/pkg/net/http/transport_test.go
@@ -778,6 +778,45 @@ func TestTransportPersistConnLeak(t *testing.T) {
}\n}\n
+// golang.org/issue/4531: Transport leaks goroutines when
+// request.ContentLength is explicitly short
+func TestTransportPersistConnLeakShortBody(t *testing.T) {
+ ts := httptest.NewServer(HandlerFunc(func(w ResponseWriter, r *Request) {
+ }))
+ defer ts.Close()
+
+ tr := &Transport{}
+ c := &Client{Transport: tr}
+
+ n0 := runtime.NumGoroutine()
+ body := []byte("Hello")
+ for i := 0; i < 20; i++ {
+ req, err := NewRequest("POST", ts.URL, bytes.NewReader(body))
+ if err != nil {
+ t.Fatal(err)
+ }
+ req.ContentLength = int64(len(body) - 2) // explicitly short
+ _, err = c.Do(req)
+ if err == nil {
+ t.Fatal("Expect an error from writing too long of a body.")
+ }
+ }
+ nhigh := runtime.NumGoroutine()
+ tr.CloseIdleConnections()
+ time.Sleep(50 * time.Millisecond)
+ runtime.GC()
+ nfinal := runtime.NumGoroutine()
+
+ growth := nfinal - n0
+
+ // We expect 0 or 1 extra goroutine, empirically. Allow up to 5.
+ // Previously we were leaking one per numReq.
+ t.Logf("goroutine growth: %d -> %d -> %d (delta: %d)", n0, nhigh, nfinal, growth)
+ if int(growth) > 5 {
+ t.Error("too many new goroutines")
+ }
+}
+
// This used to crash; http://golang.org/issue/3266
func TestTransportIdleConnCrash(t *testing.T) {
tr := &Transport{}\n
GitHub上でのコミットページへのリンク
https://github.com/golang/go/commit/7c3577e48f629120604d232c7a3994cf40ae4cda
元コミット内容
このコミットは、Go言語の標準ライブラリである net/http
パッケージにおいて、エラー発生時のゴルーチンリークを修正することを目的としています。特に、HTTPリクエストの ContentLength
ヘッダが実際のボディサイズよりも意図的に短く設定されている場合に、接続が適切にクローズされず、関連するゴルーチンがリークしてしまう問題(Issue #4531)に対処しています。この修正により、リソースの無駄な消費が抑えられ、アプリケーションの安定性が向上します。
変更の背景
この変更は、GoのIssueトラッカーで報告された #4531 に対応するものです。報告された問題は、net/http
パッケージの Transport
が、HTTPリクエストの ContentLength
が実際のボディサイズよりも小さい値に設定されている場合に、ゴルーチンをリークするというものでした。
具体的には、クライアントが ContentLength
を実際のボディサイズより短く指定してリクエストを送信すると、サーバーは指定された ContentLength
分だけボディを読み込み、残りのデータは読み飛ばされます。この際、net/http
の内部では、持続的接続(persistent connection)を管理するためのゴルーチンが起動されますが、エラーケースにおいてこの接続が適切にクローズされないことがありました。結果として、接続を待機しているゴルーチンが終了せず、リークが発生していました。
ゴルーチンリークは、時間の経過とともにシステムのリソース(メモリやCPU)を消費し続け、最終的にはアプリケーションのパフォーマンス低下やクラッシュを引き起こす可能性があります。この問題は、特に多数のHTTPリクエストを処理するサーバーアプリケーションにおいて深刻な影響を及ぼすため、早急な修正が必要とされました。
前提知識の解説
このコミットを理解するためには、以下のGo言語およびHTTPプロトコルに関する前提知識が必要です。
- ゴルーチン (Goroutine): Go言語における軽量な並行処理の単位です。OSのスレッドよりもはるかに軽量で、数百万のゴルーチンを同時に実行することも可能です。ゴルーチンはGoランタイムによってスケジューリングされ、チャネルを通じて通信します。ゴルーチンリークとは、不要になったゴルーチンが終了せずに残り続ける状態を指し、メモリやCPUリソースを無駄に消費します。
net/http
パッケージ: Go言語の標準ライブラリで、HTTPクライアントおよびサーバーの実装を提供します。Webアプリケーションの構築やHTTPリクエストの送信に広く利用されます。http.Transport
:net/http
パッケージの一部で、HTTPリクエストの実際の送信(TCP接続の確立、リクエストの書き込み、レスポンスの読み込みなど)を担当します。特に、持続的接続(Keep-Alive)の管理や、接続の再利用(コネクションプーリング)を行います。- 持続的接続 (Persistent Connection / Keep-Alive): HTTP/1.1で導入された機能で、一つのTCP接続上で複数のHTTPリクエスト/レスポンスをやり取りすることを可能にします。これにより、リクエストごとにTCP接続を確立・切断するオーバーヘッドが削減され、パフォーマンスが向上します。
http.Transport
はこの持続的接続を管理し、アイドル状態の接続を再利用します。 Content-Length
ヘッダ: HTTPリクエストまたはレスポンスのボディの長さをオクテット単位で示すヘッダです。クライアントがリクエストボディを送信する際、このヘッダを正確に設定することが重要です。サーバーはContent-Length
の値に基づいてボディを読み込みます。- エラーハンドリングとリソースクリーンアップ: プログラミングにおいて、エラーが発生した場合に、開いているファイルハンドル、ネットワーク接続、ゴルーチンなどのリソースを適切に解放(クリーンアップ)することは非常に重要です。これを怠ると、リソースリークが発生し、システムの安定性やパフォーマンスに悪影響を及ぼします。
技術的詳細
このゴルーチンリークは、net/http
パッケージの Transport
が持続的接続を管理する内部ロジックに起因していました。Transport
は、HTTPリクエストを送信し、レスポンスを受信する際に、接続を再利用するために内部的にゴルーチンを起動し、その接続の状態を監視しています。
問題が発生したのは、クライアントが POST
リクエストなどでボディを送信する際に、req.ContentLength
を実際のボディサイズよりも意図的に短い値に設定した場合です。この状況では、Transport
は ContentLength
で指定されたバイト数だけボディを書き込もうとしますが、実際のボディがそれよりも長いため、書き込み中にエラー(例えば、io.EOF
や io.ErrUnexpectedEOF
など)が発生します。
このエラーが発生した際、Transport
の内部では、writeErrCh
というチャネルを通じて書き込みエラーが通知されます。しかし、修正前のコードでは、このエラーが検出されたときに、関連する持続的接続 pc
が適切にクローズされていませんでした。持続的接続がクローズされないと、その接続を管理しているゴルーチン(通常は接続からの読み込みや書き込みを待機している)が終了せず、アイドル状態のまま残り続けてしまいます。これがゴルーチンリークの原因でした。
リークしたゴルーチンは、実際には何も処理を行っていないにもかかわらず、メモリを消費し続け、Goランタイムのスケジューリングにも影響を与える可能性があります。特に、このような不正な ContentLength
を持つリクエストが多数送信されると、リークするゴルーチンの数も増大し、システム全体のリソースが枯渇する事態に陥る可能性がありました。
このコミットでは、writeErrCh
からエラーが受信された場合に、即座に pc.close()
を呼び出すことで、この問題を解決しています。これにより、エラー発生時に持続的接続が強制的にクローズされ、関連するゴルーチンも適切に終了するようになります。
コアとなるコードの変更箇所
このコミットにおけるコアとなるコードの変更は、以下の2つのファイルにわたります。
-
src/pkg/net/http/transport.go
:WaitResponse:
ラベル内のcase err := <-writeErrCh:
ブロックに、pc.close()
の呼び出しが追加されました。
--- a/src/pkg/net/http/transport.go +++ b/src/pkg/net/http/transport.go @@ -742,6 +742,7 @@ WaitResponse: case err := <-writeErrCh: if err != nil { re = responseAndError{nil, err} + pc.close() break WaitResponse } case <-pconnDeadCh:
-
src/pkg/net/http/transport_test.go
:TestTransportPersistConnLeakShortBody
という新しいテスト関数が追加されました。このテストは、ContentLength
が意図的に短く設定されたリクエストを複数回送信し、ゴルーチンリークが発生しないことを検証します。
// golang.org/issue/4531: Transport leaks goroutines when // request.ContentLength is explicitly short func TestTransportPersistConnLeakShortBody(t *testing.T) { ts := httptest.NewServer(HandlerFunc(func(w ResponseWriter, r *Request) { })) defer ts.Close() tr := &Transport{} c := &Client{Transport: tr} n0 := runtime.NumGoroutine() body := []byte("Hello") for i := 0; i < 20; i++ { req, err := NewRequest("POST", ts.URL, bytes.NewReader(body)) if err != nil { t.Fatal(err) } req.ContentLength = int64(len(body) - 2) // explicitly short _, err = c.Do(req) if err == nil { t.Fatal("Expect an error from writing too long of a body.") } } nhigh := runtime.NumGoroutine() tr.CloseIdleConnections() time.Sleep(50 * time.Millisecond) runtime.GC() nfinal := runtime.NumGoroutine() growth := nfinal - n0 // We expect 0 or 1 extra goroutine, empirically. Allow up to 5. // Previously we were leaking one per numReq. t.Logf("goroutine growth: %d -> %d -> %d (delta: %d)", n0, nhigh, nfinal, growth) if int(growth) > 5 { t.Error("too many new goroutines") } }
コアとなるコードの解説
src/pkg/net/http/transport.go
の変更
transport.go
の変更は非常にシンプルですが、ゴルーチンリークの修正において最も重要な部分です。
case err := <-writeErrCh:
if err != nil {
re = responseAndError{nil, err}
pc.close() // 追加された行
break WaitResponse
}
このコードスニペットは、Transport
の内部でHTTPリクエストの書き込み処理中にエラーが発生した場合のハンドリングを示しています。
writeErrCh
は、リクエストボディの書き込み中に発生したエラーを通知するためのチャネルです。err := <-writeErrCh
は、このチャネルからエラーを受信することを意味します。if err != nil
は、実際にエラーが発生した場合の処理ブロックです。re = responseAndError{nil, err}
は、レスポンスとエラーの構造体にエラー情報を設定しています。pc.close()
: ここが今回の修正の核心です。pc
はpersistConn
型の変数で、持続的接続を表します。以前のコードでは、書き込みエラーが発生してもこの接続が明示的にクローズされていませんでした。pc.close()
を呼び出すことで、エラーが発生した際にこの持続的接続が強制的に閉じられます。これにより、この接続を管理していた内部のゴルーチン(例えば、接続からの読み込みを待機していたゴルーチン)が適切に終了し、リークが防止されます。break WaitResponse
は、エラーが発生したため、レスポンスの待機ループを終了することを示します。
この一行の追加により、エラーパスにおいても持続的接続のリソースが確実に解放されるようになり、関連するゴルーチンがリークする問題が解決されました。
src/pkg/net/http/transport_test.go
の変更
TestTransportPersistConnLeakShortBody
テスト関数は、この修正が正しく機能することを検証するために追加されました。
-
テストサーバーのセットアップ:
httptest.NewServer
を使用して、ダミーのHTTPサーバーを起動します。このサーバーは、リクエストを受け取っても特に何も処理を行いません。 -
Transport
とClient
の初期化:tr := &Transport{}
とc := &Client{Transport: tr}
で、テスト用のTransport
とClient
を作成します。 -
ゴルーチン数の初期値取得:
n0 := runtime.NumGoroutine()
で、テスト開始前のシステム全体のゴルーチン数を記録します。これは、リークが発生したかどうかを判断するためのベースラインとなります。 -
意図的に短い
ContentLength
のリクエスト送信:for i := 0; i < 20; i++
ループで、20回HTTPリクエストを送信します。body := []byte("Hello")
で、実際のボディデータを作成します。req.ContentLength = int64(len(body) - 2)
: ここがテストの肝です。実際のボディ長 (len(body)
) よりも2バイト短いContentLength
を明示的に設定します。これにより、Transport
がボディを書き込む際にエラーが発生する状況を意図的に作り出します。_, err = c.Do(req)
: リクエストを実行します。ContentLength
が短いことによる書き込みエラーが期待されるため、err
がnil
でないことを確認します。もしerr == nil
であれば、テストは失敗します。
-
ゴルーチン数の確認とクリーンアップ:
nhigh := runtime.NumGoroutine()
: リクエスト送信後のゴルーチン数を記録します。tr.CloseIdleConnections()
:Transport
が管理するアイドル状態の持続的接続をすべてクローズします。これにより、テスト終了前に可能な限り多くのゴルーチンが終了するように促します。time.Sleep(50 * time.Millisecond)
とruntime.GC()
: ゴルーチンが終了し、ガベージコレクションが実行されるのを待つための短い遅延と明示的なGC呼び出しです。nfinal := runtime.NumGoroutine()
: 最終的なゴルーチン数を記録します。
-
リークの検証:
growth := nfinal - n0
: テスト開始時と終了時のゴルーチン数の差を計算します。if int(growth) > 5
: ゴルーチン数の増加が5を超えている場合、リークが発生していると判断し、テストを失敗させます。コメントにあるように、以前はリクエストごとに1つのゴルーチンがリークしていたため、この閾値が設定されています。
このテストは、修正がゴルーチンリークを効果的に防いでいることを、具体的なシナリオで検証しています。
関連リンク
- Go Issue #4531: https://golang.org/issue/4531
- Go Code Review 6937069: https://golang.org/cl/6937069