[インデックス 14893] ファイルの概要
このコミットは、Go言語の標準ライブラリであるnet/httpパッケージ内のテストファイルsrc/pkg/net/http/serve_test.goに対する修正です。具体的には、HTTPハンドラの実行完了を待機するテストのロジックに存在していた競合状態（race condition）を修正し、テストの信頼性を向上させることを目的としています。
コミット
- コミットハッシュ:
3b73aaafdcd35ad20329730f5193859f491e59f4 - 作者: Brad Fitzpatrick bradfitz@golang.org
- 日付: 2013年1月15日 火曜日 09:13:05 -0800
GitHub上でのコミットページへのリンク
https://github.com/golang/go/commit/3b73aaafdcd35ad20329730f5193859f491e59f4
元コミット内容
net/http: fix racy test
We need to wait for the handler to actually finish running,
not almost be done running.
This was always a bug, but now that handler output is buffered
it shows up easily on GOMAXPROCS >1 systems.
R=golang-dev, iant
CC=golang-dev
https://golang.org/cl/7109043
変更の背景
このコミットの背景には、net/httpパッケージのテストスイートにおける既存の競合状態のバグがあります。以前のテストでは、HTTPハンドラがその処理を「ほぼ完了した」時点でテストがハンドラの終了を検知し、次のステップに進んでいました。しかし、これはハンドラが実際に完全に終了したことを保証するものではありませんでした。
特に、Go 1.1で導入されたGOMAXPROCS環境変数（Goランタイムが使用するOSスレッドの最大数を制御する）が1より大きいシステム、つまり複数のCPUコアやスレッドを利用できる環境では、この競合状態が顕在化しやすくなりました。複数のゴルーチンが並行して動作する環境では、ハンドラの処理とテストの終了検知のタイミングがずれる可能性が高まります。
さらに、ハンドラの出力がバッファリングされるようになったことも、この問題の表面化を助長しました。出力がバッファリングされると、ハンドラがResponseWriterに書き込みを終えたとしても、そのデータがすぐにネットワークにフラッシュされるわけではありません。テストがハンドラの出力バッファが空になったことをもってハンドラの終了と判断していた場合、実際のハンドラのロジックがまだ実行中であるにもかかわらず、テストが終了してしまうという状況が発生しやすくなりました。
この修正は、ハンドラの完全な実行完了を確実に待機することで、テストの信頼性と再現性を向上させることを目的としています。
前提知識の解説
- Go言語の
net/httpパッケージ: Go言語でHTTPクライアントおよびサーバーを構築するための標準ライブラリです。http.Handlerインターフェースを実装することで、HTTPリクエストを処理するハンドラを作成できます。http.Serve関数は、指定されたリスナーとハンドラを使用してHTTPサーバーを起動します。 http.ResponseWriterとhttp.Request:http.HandlerインターフェースのServeHTTPメソッドに渡される引数です。http.ResponseWriterはHTTPレスポンスを書き込むためのインターフェースであり、http.Requestは受信したHTTPリクエストの詳細を含みます。testConn構造体: このテストファイル内で定義されているカスタムのネットワーク接続を模倣する構造体です。実際のネットワークI/Oを行わず、bytes.Bufferを使用して読み書きをシミュレートします。これにより、テストの制御性と再現性が向上します。bytes.Buffer:bytesパッケージに含まれる可変長のバイトバッファです。バイト列の読み書きを効率的に行えます。testConn内で、擬似的なネットワーク接続の送受信バッファとして使用されます。chan bool(チャネル): Go言語におけるゴルーチン間の通信と同期のためのプリミティブです。chan boolはbool型の値を送受信できるチャネルを意味します。このコミットでは、ゴルーチン（ハンドラ）の完了を別のゴルーチン（テスト本体）に通知するために使用されます。- 競合状態 (Race Condition): 複数のゴルーチン（またはスレッド）が共有リソースに同時にアクセスし、そのアクセス順序によってプログラムの最終結果が変わってしまうバグのことです。テストにおいて競合状態が存在すると、テストが不安定になり、特定の環境や実行タイミングでのみ失敗する「flaky test」となります。
GOMAXPROCS: Goランタイムが同時に実行できるOSスレッドの最大数を設定する環境変数です。デフォルトではCPUコア数に設定されます。GOMAXPROCS > 1の場合、Goランタイムは複数のOSスレッドを使用してゴルーチンを並行実行しようとします。これにより、競合状態が顕在化しやすくなります。deferステートメント: Go言語のキーワードで、関数がリターンする直前に実行される関数呼び出しをスケジュールします。通常、リソースの解放（ファイルのクローズ、ロックの解除など）に使用されます。selectステートメント: 複数のチャネル操作を待機し、準備ができた最初の操作を実行します。defaultケースを含めることで、どのチャネル操作も準備ができていない場合にすぐに実行される非ブロッキングな操作を定義できます。
技術的詳細
このコミットの核心は、net/httpテストにおけるハンドラの終了検知メカニズムの改善です。以前のテストでは、ハンドラが終了したことを示すためにdoneチャネルが使用されていましたが、このチャネルはハンドラの処理が完全に終わる前に閉じられる可能性がありました。
具体的な問題点は以下の通りです。
defer close(done)の不適切性: 以前のコードでは、HandlerFuncのゴルーチン内でdefer close(done)が使用されていました。deferは関数がリターンする直前に実行されますが、ハンドラがResponseWriterへの書き込みを終えたとしても、その後の内部的な処理（例えば、バッファのフラッシュや接続のクローズ）がまだ完了していない可能性があります。特に、ハンドラの出力がバッファリングされるようになったことで、Write呼び出しがすぐに完了しても、実際のデータ送信は遅延する可能性があります。テストが<-doneでハンドラの終了を待つ場合、ハンドラが完全にクリーンアップされる前にテストが続行され、競合状態が発生していました。GOMAXPROCS > 1での顕在化: 複数のCPUコアが利用可能な環境では、ハンドラを実行するゴルーチンとテスト本体を実行するゴルーチンが並行して動作します。これにより、ハンドラがdoneチャネルを閉じるタイミングと、テストがそのチャネルから読み取ろうとするタイミングの間に、より多くのタイミングのずれが生じやすくなり、競合状態が頻繁に発生するようになりました。
この修正では、testConn構造体にclosec chan boolという新しいチャネルを追加し、Close()メソッドが呼び出されたときにこのチャネルに値を送信するように変更しました。
testConn.closecの導入:testConnは擬似的なネットワーク接続を表します。この接続がクローズされる（つまり、HTTPサーバーがクライアントとの通信を終了する）タイミングで、closecチャネルにtrueが送信されます。Close()メソッドでのselectとdefault:testConnのClose()メソッド内でselect { case c.closec <- true: default: }というパターンが使用されています。これは、closecチャネルに値を送信しようとしますが、もしチャネルが準備できていない（例えば、バッファがいっぱいであるか、受信側がまだ準備できていない）場合でも、ブロックせずにすぐにdefaultケースが実行されることを意味します。これにより、Close()メソッド自体がブロックされることなく、チャネルへの送信が試みられます。このclosecはバッファ付きチャネル（make(chan bool, 1)）として作成されるため、通常はブロックせずに送信が成功します。- テストでの待機メカニズムの変更:
- 以前の
done := make(chan bool)とdefer close(done)のペアは削除されました。 - 代わりに、
conn.closec = make(chan bool, 1)としてtestConnにバッファ付きチャネルを割り当てます。 - テストの終了待機は
<-doneから<-conn.closecに変更されました。
- 以前の
この変更により、テストはHTTPハンドラが単に処理を終えるだけでなく、そのハンドラが使用していた擬似的なネットワーク接続（testConn）が実際にクローズされるまで待機するようになります。接続のクローズは、HTTPサーバーがクライアントへのレスポンス送信を完了し、関連するリソースを解放したことを示す、より確実な指標となります。これにより、テストの競合状態が解消され、より安定したテスト実行が保証されます。
コアとなるコードの変更箇所
diff --git a/src/pkg/net/http/serve_test.go b/src/pkg/net/http/serve_test.go
index 96d442b623..853aac7f4d 100644
--- a/src/pkg/net/http/serve_test.go
+++ b/src/pkg/net/http/serve_test.go
@@ -67,6 +67,7 @@ func (a dummyAddr) String() string {
type testConn struct {
readBuf bytes.Buffer
writeBuf bytes.Buffer
+ closec chan bool // if non-nil, send value to it on close
}
func (c *testConn) Read(b []byte) (int, error) {
@@ -78,6 +79,10 @@ func (c *testConn) Write(b []byte) (int, error) {
}
func (c *testConn) Close() error {
+ select {
+ case c.closec <- true:
+ default:
+ }
return nil
}
@@ -788,12 +793,10 @@ func TestServerUnreadRequestBodyLarge(t *testing.T) {
"Content-Length: %d\r\n"+
"\r\n", len(body))))
conn.readBuf.Write([]byte(body))
-
- done := make(chan bool)
+ conn.closec = make(chan bool, 1)
ls := &oneConnListener{conn}
go Serve(ls, HandlerFunc(func(rw ResponseWriter, req *Request) {
- defer close(done)
if conn.readBuf.Len() < len(body)/2 {
t.Errorf("on request, read buffer length is %d; expected about 1MB", conn.readBuf.Len())
}
@@ -803,7 +806,7 @@ func TestServerUnreadRequestBodyLarge(t *testing.T) {
t.Errorf("post-WriteHeader, read buffer length is %d; expected about 1MB", conn.readBuf.Len())
}
}))
- <-done
+ <-conn.closec
if res := conn.writeBuf.String(); !strings.Contains(res, "Connection: close") {
t.Errorf("Expected a Connection: close header; got response: %s", res)
@@ -1150,16 +1153,15 @@ func TestClientWriteShutdown(t *testing.T) {
func TestServerBufferedChunking(t *testing.T) {\n \tconn := new(testConn)\n \tconn.readBuf.Write([]byte(\"GET / HTTP/1.1\\r\\n\\r\\n\"))\n-\tdone := make(chan bool)\n+\tconn.closec = make(chan bool, 1)\n \tls := &oneConnListener{conn}\n \tgo Serve(ls, HandlerFunc(func(rw ResponseWriter, req *Request) {\n-\t\tdefer close(done)\n \t\trw.(Flusher).Flush() // force the Header to be sent, in chunking mode, not counting the length\n \t\trw.Write([]byte{\'x\'})\n \t\trw.Write([]byte{\'y\'})\n \t\trw.Write([]byte{\'z\'})\n \t}))\n-\t<-done\n+\t<-conn.closec\n \tif !bytes.HasSuffix(conn.writeBuf.Bytes(), []byte(\"\\r\\n\\r\\n3\\r\\nxyz\\r\\n0\\r\\n\\r\\n\")) {\n \t\tt.Errorf(\"response didn\'t end with a single 3 byte \'xyz\' chunk; got:\\n%q\",\n \t\t\tconn.writeBuf.Bytes())\n```
## コアとなるコードの解説
このコミットは、主に`src/pkg/net/http/serve_test.go`ファイル内の`testConn`構造体と、それを使用するテスト関数`TestServerUnreadRequestBodyLarge`および`TestServerBufferedChunking`に変更を加えています。
1. **`testConn`構造体への`closec`フィールドの追加**:
```diff
@@ -67,6 +67,7 @@ func (a dummyAddr) String() string {
type testConn struct {
readBuf bytes.Buffer
writeBuf bytes.Buffer
+ closec chan bool // if non-nil, send value to it on close
}
```
`testConn`は、テスト目的でネットワーク接続をシミュレートするための構造体です。ここに`closec chan bool`という新しいフィールドが追加されました。このチャネルは、`testConn`がクローズされたときに、そのイベントを通知するために使用されます。コメントにあるように、`nil`でない場合に値を送信します。
2. **`testConn.Close()`メソッドの変更**:
```diff
@@ -78,6 +79,10 @@ func (c *testConn) Write(b []byte) (int, error) {
}
func (c *testConn) Close() error {
+ select {\n+\tcase c.closec <- true:\n+\tdefault:\n+\t}\n return nil
}
```
`testConn`の`Close()`メソッドは、擬似的な接続が閉じられたときに呼び出されます。この変更により、`closec`チャネルが`nil`でない場合（つまり、テストで初期化されている場合）、`true`の値をそのチャネルに送信しようとします。
`select { case c.closec <- true: default: }`のパターンは重要です。これは非ブロッキング送信を意味します。もし`closec`チャネルがバッファリングされていて、まだ空きがある場合、または受信側が準備できている場合、`true`が送信されます。そうでない場合（例えば、チャネルがバッファなしで、受信側がまだ準備できていない場合）、`default`ケースが実行され、`Close()`メソッドはブロックせずにすぐにリターンします。これにより、`Close()`操作がチャネルの送信によって遅延することがなくなります。
3. **`TestServerUnreadRequestBodyLarge`テスト関数の変更**:
```diff
@@ -788,12 +793,10 @@ func TestServerUnreadRequestBodyLarge(t *testing.T) {
"Content-Length: %d\r\n"+
"\r\n", len(body))))
conn.readBuf.Write([]byte(body))
-
- done := make(chan bool)
+ conn.closec = make(chan bool, 1)
ls := &oneConnListener{conn}
go Serve(ls, HandlerFunc(func(rw ResponseWriter, req *Request) {
- defer close(done)
if conn.readBuf.Len() < len(body)/2 {
t.Errorf("on request, read buffer length is %d; expected about 1MB", conn.readBuf.Len())
}
@@ -803,7 +806,7 @@ func TestServerUnreadRequestBodyLarge(t *testing.T) {
t.Errorf("post-WriteHeader, read buffer length is %d; expected about 1MB", conn.readBuf.Len())
}
}))
- <-done
+ <-conn.closec
if res := conn.writeBuf.String(); !strings.Contains(res, "Connection: close") {
t.Errorf("Expected a Connection: close header; got response: %s", res)
```
* `done := make(chan bool)`の行が削除されました。これは、ハンドラの完了を通知するために以前使用されていたチャネルです。
* `conn.closec = make(chan bool, 1)`が追加されました。これにより、`testConn`インスタンスにバッファ付き（バッファサイズ1）の`closec`チャネルが割り当てられます。このチャネルは、`testConn.Close()`が呼び出されたときに通知を受け取るために使用されます。
* `defer close(done)`の行が削除されました。これにより、ハンドラがリターンする直前に`done`チャネルを閉じるという、競合状態の原因となっていた動作がなくなります。
* `<-done`の行が`<-conn.closec`に置き換えられました。テストは、ハンドラが終了したことを示す`done`チャネルを待つ代わりに、`testConn`がクローズされるまで待機するようになりました。これは、HTTPサーバーがレスポンスの送信を完了し、接続をクリーンアップしたことをより正確に示します。
4. **`TestServerBufferedChunking`テスト関数の変更**:
```diff
@@ -1150,16 +1153,15 @@ func TestClientWriteShutdown(t *testing.T) {
func TestServerBufferedChunking(t *testing.T) {
conn := new(testConn)
conn.readBuf.Write([]byte("GET / HTTP/1.1\r\n\r\n"))
- done := make(chan bool)
+ conn.closec = make(chan bool, 1)
ls := &oneConnListener{conn}
go Serve(ls, HandlerFunc(func(rw ResponseWriter, req *Request) {
- defer close(done)
rw.(Flusher).Flush() // force the Header to be sent, in chunking mode, not counting the length
rw.Write([]byte{'x'})
rw.Write([]byte{'y'})
rw.Write([]byte{'z'})
}))
- <-done
+ <-conn.closec
if !bytes.HasSuffix(conn.writeBuf.Bytes(), []byte("\r\n\r\n3\r\nxyz\r\n0\r\n\r\n")) {
t.Errorf("response didn't end with a single 3 byte 'xyz' chunk; got:\n%q",
conn.writeBuf.Bytes())
```
このテスト関数も`TestServerUnreadRequestBodyLarge`と同様の変更が適用されています。`done`チャネルの使用が削除され、`testConn.closec`チャネルを介して接続のクローズを待機するように変更されました。これにより、チャンクされたレスポンスのテストにおいても、ハンドラの完全な終了が保証されます。
これらの変更により、テストはハンドラの内部処理が完了し、かつそのハンドラが使用していたネットワーク接続が適切にクローズされるまで待機するようになり、テストの信頼性と安定性が大幅に向上しました。
## 関連リンク
* **Go Code Review 7109043**: このコミットの元となったGoのコードレビューシステム（Gerrit）の変更リストです。詳細な議論やレビューコメントが含まれている可能性があります。
* [https://go-review.googlesource.com/c/go/+/7109043](https://go-review.googlesource.com/c/go/+/7109043)
* **Go言語の並行処理と競合状態**: Go言語における並行処理の概念、チャネルの利用、および競合状態の検出と回避に関する一般的な情報源。
* [https://go.dev/doc/effective_go#concurrency](https://go.dev/doc/effective_go#concurrency)
* [https://go.dev/blog/race-detector](https://go.dev/blog/race-detector) (Go Race Detectorに関するブログ記事)
* **Go言語の`net/http`パッケージドキュメント**:
* [https://pkg.go.dev/net/http](https://pkg.go.dev/net/http)
## 参考にした情報源リンク
* Go Code Review: [https://go-review.googlesource.com/c/go/+/7109043](https://go-review.googlesource.com/c/go/+/7109043)
* Go言語公式ドキュメント: [https://go.dev/doc/](https://go.dev/doc/)
* Go言語の`net/http`パッケージドキュメント: [https://pkg.go.dev/net/http](https://pkg.go.dev/net/http)
* Go言語の`bytes`パッケージドキュメント: [https://pkg.go.dev/bytes](https://pkg.go.dev/bytes)
* Go言語のチャネルに関する情報 (Effective Goなど): [https://go.dev/doc/effective_go#concurrency](https://go.dev/doc/effective_go#concurrency)
* Go Race Detectorに関する情報: [https://go.dev/blog/race-detector](https://go.dev/blog/race-detector)
* `GOMAXPROCS`に関する情報: [https://go.dev/doc/go1.1#gomaxprocs](https://go.dev/doc/go1.1#gomaxprocs) (Go 1.1リリースノート)