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

このコミットは、Go言語の標準ライブラリであるnet/httpパッケージ内のテストコードにおいて、Goのレース検出器（Race Detector）が報告するデータ競合（data race）を解消するための修正です。具体的には、TestTransportClosesBodyOnErrorというテスト関数で発生していた競合状態を、チャネルを用いた同期メカニズムによって回避しています。

コミット

commit 172acae68ae4c89190df48a7cec084d6cc27c49d
Author: Brad Fitzpatrick <bradfitz@golang.org>
Date:   Mon Apr 14 12:08:32 2014 -0700

    net/http: make race detector happy for recently-added test
    
    Update #7264
    
    Races:
    http://build.golang.org/log/a2e401fdcd4903a61a3375bff5da702a20ddafad
    http://build.golang.org/log/ec4c69e92076a747ac6d5df7eb7b382b31ab3d43
    
    I think this is the first time I've actually seen a manifestation
    of Issue 7264, and one that I can reproduce.
    
    I don't know why it triggers on this test and not any others
    just like it, or why I can't reproduce Issue 7264
    independently, even when Dmitry gives me minimal repros.
    
    Work around it for now with some synchronization to make the
    race detector happy.
    
    The proper fix will probably be in net/http/httptest itself, not
    in all hundred some tests.
    
    LGTM=rsc
    R=rsc
    CC=dvyukov, golang-codereviews
    https://golang.org/cl/87640043

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

https://github.com/golang/go/commit/172acae68ae4c89190df48a7cec084d6cc27c49d

元コミット内容

net/http: make race detector happy for recently-added test

Update #7264

Races:
http://build.golang.org/log/a2e401fdcd4903a61a3375bff5da702a20ddafad
http://build.golang.org/log/ec4c69e92076a747ac6d5df7eb7b382b31ab3d43

I think this is the first time I've actually seen a manifestation
of Issue 7264, and one that I can reproduce.

I don't know why it triggers on this test and not any others
just like it, or why I can't reproduce Issue 7264
independently, even when Dmitry gives me minimal repros.

Work around it for now with some synchronization to make the
race detector happy.

The proper fix will probably be in net/http/httptest itself, not
in all hundred some tests.

LGTM=rsc
R=rsc
CC=dvyukov, golang-codereviews
https://golang.org/cl/87640043

変更の背景

このコミットの背景には、Goのレース検出器（Race Detector）がnet/httpパッケージのTestTransportClosesBodyOnErrorというテストでデータ競合を検出したという問題があります。コミットメッセージによると、これはIssue 7264に関連する問題の具体的な現れであり、コミッターのBrad Fitzpatrick氏が初めて再現できたケースであったようです。

データ競合は、複数のゴルーチンが同時に同じメモリ位置にアクセスし、少なくとも1つのアクセスが書き込みである場合に発生します。このような競合はプログラムの予測不能な動作やクラッシュを引き起こす可能性があり、Goのレース検出器は開発者がこれらの問題を特定するのに役立つ強力なツールです。

この特定のテストでは、HTTPリクエストのボディを読み取る処理と、テストのメインゴルーチンでの処理との間に同期が不足していたため、レース検出器が警告を発していました。コミッターは、この問題の根本的な解決策はnet/http/httptestパッケージ自体にあると考えていますが、当面のワークアラウンドとして、テストコードに同期メカニズムを追加することでレース検出器の警告を解消することを選択しました。これは、テストがCI（継続的インテグレーション）システムで失敗するのを防ぎ、開発プロセスを円滑に進めるための実用的なアプローチです。

前提知識の解説

1. Goの並行処理とゴルーチン (Goroutines)

Go言語は、軽量な並行処理のプリミティブとして「ゴルーチン（goroutine）」を提供します。ゴルーチンはOSのスレッドよりもはるかに軽量で、数千、数万のゴルーチンを同時に実行することが可能です。ゴルーチン間の通信には主に「チャネル（channel）」が用いられ、これにより安全なデータ共有と同期が実現されます（"Don't communicate by sharing memory; share memory by communicating." というGoの哲学）。

2. データ競合 (Data Race)

データ競合は、並行プログラミングにおける一般的なバグの一種です。以下の3つの条件がすべて満たされたときに発生します。

• 少なくとも2つのゴルーチンが同じメモリ位置にアクセスする。
• 少なくとも1つのアクセスが書き込み操作である。
• これらのアクセスが同期メカニズムなしに同時に行われる。 データ競合が発生すると、プログラムの動作が非決定論的になり、予期せぬ結果やクラッシュにつながる可能性があります。

3. Goのレース検出器 (Race Detector)

Goには、データ競合を検出するための組み込みツールである「レース検出器」があります。これは、プログラムの実行中にメモリアクセスを監視し、データ競合のパターンを検出すると警告を発します。go run -race、go build -race、go test -raceなどのコマンドで有効にできます。レース検出器は、本番環境での実行にはオーバーヘッドが大きいですが、開発およびテスト段階で並行処理のバグを特定するのに非常に有効です。

4. net/httpパッケージ

Goの標準ライブラリであるnet/httpパッケージは、HTTPクライアントとサーバーの実装を提供します。WebアプリケーションやAPIの構築に不可欠なパッケージです。このパッケージは、内部的に多くの並行処理を利用しており、ゴルーチンとチャネルを効果的に使用してリクエストの処理やコネクションの管理を行っています。

5. net/http/httptestパッケージ

net/http/httptestパッケージは、HTTPサーバーとクライアントのテストを容易にするためのユーティリティを提供します。httptest.NewServer関数は、テスト中に実際のHTTPサーバーを起動し、テスト対象のHTTPハンドラを検証するために使用されます。これにより、ネットワークI/Oを伴うテストを、実際のネットワークに依存せずに実行できます。

6. io/ioutilパッケージ (Go 1.16以降はioおよびosパッケージに統合)

io/ioutilパッケージは、I/O操作に関するユーティリティ関数を提供していました。このコミットが作成された時点では、ioutil.ReadAllはio.Readerからすべてのデータを読み取り、バイトスライスとして返す関数でした。Go 1.16以降では、この機能はio.ReadAllに移行しています。

技術的詳細

このコミットは、src/pkg/net/http/transport_test.goファイル内のTestTransportClosesBodyOnErrorテスト関数に焦点を当てています。このテストは、HTTPトランスポートがエラー発生時にリクエストボディを適切にクローズするかどうかを検証するものです。

元のコードでは、httptest.NewServerで起動されるテストサーバーのハンドラ内で、ioutil.ReadAll(r.Body)が呼び出されていました。このReadAllの呼び出しは、リクエストボディの読み込みを試みますが、テストの性質上、クライアント側で意図的にエラーを発生させているため、ボディの読み込みが途中で中断される可能性があります。

問題は、テストのメインゴルーチンと、httptest.NewServerによって起動されたハンドラゴルーチン（リクエストを処理するゴルーチン）との間で、r.Bodyの読み込み完了状態に関する同期が不足していたことです。レース検出器は、r.Bodyへのアクセス（読み込み）と、テストのメインゴルーチンがr.Bodyに関連する状態をチェックするタイミングとの間に競合を検出しました。具体的には、ioutil.ReadAllがエラーを返すか、または正常に完了したかどうかの情報が、テストのメインゴルーチンに安全に伝達されていなかったため、レース検出器が誤検知（または実際の競合）を報告していました。

この修正では、Goのチャネル（chan error）を導入することで、この同期の問題を解決しています。

1. readBody := make(chan error, 1): バッファ付きチャネル（容量1）readBodyが作成されます。これは、ハンドラゴルーチンからメインゴルーチンへエラー情報を安全に送信するために使用されます。バッファ付きチャネルであるため、ハンドラゴルーチンは受信側が準備できるのを待たずに値を送信できます（ただし1つまで）。
2. _, err := ioutil.ReadAll(r.Body): ハンドラ内でリクエストボディを読み込み、その結果のエラーをerr変数に格納します。
3. readBody <- err: 読み込み結果のエラーをreadBodyチャネルに送信します。これにより、ハンドラゴルーチンはボディの読み込みを完了したことをメインゴルーチンに通知します。
4. select { case err := <-readBody: ... case <-time.After(5 * time.Second): ... }: テストのメインゴルーチンでは、readBodyチャネルからの受信をselectステートメントで待ちます。これにより、ハンドラゴルーチンがボディの読み込みを完了し、結果をチャネルに送信するまで、メインゴルーチンはブロックされます。タイムアウトも設定されており、ハンドラが応答しない場合のデッドロックを防ぎます。

この同期メカニズムにより、r.Bodyへのアクセスが完了し、その結果が安全に伝達された後にのみ、テストのメインゴルーチンが次の処理に進むことが保証されます。これにより、レース検出器が報告していたデータ競合が解消されます。コミットメッセージにあるように、これは根本的な解決策ではなく、テスト固有のワークアラウンドですが、CIシステムでのテストの安定性を確保するためには有効な手段です。

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

変更はsrc/pkg/net/http/transport_test.goファイル内のTestTransportClosesBodyOnError関数に集中しています。

--- a/src/pkg/net/http/transport_test.go
+++ b/src/pkg/net/http/transport_test.go
@@ -2041,8 +2041,10 @@ func (f closerFunc) Close() error { return f() }\n // Issue 6981
 func TestTransportClosesBodyOnError(t *testing.T) {
 	defer afterTest(t)\n+\treadBody := make(chan error, 1)\n \tts := httptest.NewServer(HandlerFunc(func(w ResponseWriter, r *Request) {
-\t\tioutil.ReadAll(r.Body)\n+\t\t_, err := ioutil.ReadAll(r.Body)\n+\t\treadBody <- err
 \t}))\n \tdefer ts.Close()\n \tfakeErr := errors.New("fake error")\n@@ -2068,6 +2070,14 @@ func TestTransportClosesBodyOnError(t *testing.T) {\n \t\tt.Fatalf("Do error = %v; want something containing %q", fakeErr.Error())\n \t}\n \tselect {\n+\tcase err := <-readBody:\n+\t\tif err == nil {\n+\t\t\tt.Errorf("Unexpected success reading request body from handler; want 'unexpected EOF reading trailer'")\n+\t\t}\n+\tcase <-time.After(5 * time.Second):\n+\t\tt.Error("timeout waiting for server handler to complete")\n+\t}\n+\tselect {\n \tcase <-didClose:\n \tdefault:\
 \t\tt.Errorf("didn't see Body.Close")\

コアとなるコードの解説

追加されたコード

1. readBody := make(chan error, 1):

   • TestTransportClosesBodyOnError関数の冒頭で、error型の値を送受信するためのバッファ付きチャネルreadBodyが作成されます。バッファサイズが1であるため、送信側は受信側が準備できるのを待たずに1つの値を送信できます。これは、HTTPハンドラがボディの読み込みを完了した直後に結果を送信し、テストのメインゴルーチンが後でその結果を受け取ることを可能にします。

2. _, err := ioutil.ReadAll(r.Body):

   • httptest.NewServerに渡されるHandlerFunc内で、以前は単にioutil.ReadAll(r.Body)が呼び出されていましたが、その戻り値（読み込んだバイト数とエラー）を受け取るように変更されました。読み込んだバイト数は不要なため、_で破棄されています。

3. readBody <- err:

   • ioutil.ReadAllの呼び出し直後に、読み込み操作の結果得られたerrをreadBodyチャネルに送信しています。これにより、ハンドラゴルーチンがボディの読み込みを完了したことをテストのメインゴルーチンに通知します。

4. 最初のselectブロック:

   select {
   case err := <-readBody:
       if err == nil {
           t.Errorf("Unexpected success reading request body from handler; want 'unexpected EOF reading trailer'")
       }
   case <-time.After(5 * time.Second):
       t.Error("timeout waiting for server handler to complete")
   }
   

   • これはテストのメインゴルーチンに追加された同期ポイントです。
   • case err := <-readBody:: readBodyチャネルから値を受信するまでブロックします。これにより、ハンドラゴルーチンがボディの読み込みを完了し、結果を送信するまでテストの実行が一時停止されます。受信したエラーがnil（読み込み成功）の場合、それは予期しない動作であるため、テストエラーとして報告されます（このテストはエラーケースを検証しているため）。
   • case <-time.After(5 * Second):: 5秒のタイムアウトを設定しています。もしハンドラゴルーチンが5秒以内にreadBodyチャネルに何も送信しなかった場合、タイムアウトエラーが報告され、テストがハングアップするのを防ぎます。

変更の意図

この変更の主な意図は、TestTransportClosesBodyOnErrorテストにおけるデータ競合を解消し、Goのレース検出器がクリーンな状態を報告するようにすることです。

• 同期の確立: readBodyチャネルを介して、HTTPハンドラゴルーチンとテストのメインゴルーチンとの間に明確な同期ポイントが確立されました。これにより、メインゴルーチンがr.Bodyに関連するアサーションを行う前に、ハンドラゴルーチンがr.Bodyの読み込みを完了したことが保証されます。
• レース検出器の満足: レース検出器は、共有メモリへのアクセスが適切に同期されていることを確認します。このチャネルベースの同期により、r.Bodyへのアクセスが順序付けられ、競合が解消されます。
• テストの堅牢性向上: タイムアウトを追加することで、ハンドラゴルーチンが何らかの理由で応答しなかった場合にテストが無限に待機するのを防ぎ、テストの堅牢性が向上しています。

この修正は、特定のテストにおけるデータ競合の症状を緩和するためのワークアラウンドであり、コミットメッセージにもあるように、net/http/httptestパッケージ自体のより根本的な修正が必要である可能性が示唆されています。しかし、これによりCIシステムでのテストの安定性が確保され、開発の妨げとなるレース検出器の警告が一時的に解消されました。

関連リンク

• Go Issue 7264: https://github.com/golang/go/issues/7264 (コミットメッセージで参照されているIssue)
• Go Race Detector Documentation: https://go.dev/doc/articles/race_detector
• Go Concurrency Patterns: Pipelines and cancellation: https://go.dev/blog/pipelines (チャネルを用いた並行処理のパターンに関するGo公式ブログ)

参考にした情報源リンク

• Go言語の公式ドキュメント
• Goのソースコード（net/httpおよびnet/http/httptestパッケージ）
• GoのIssueトラッカー (GitHub)
• Go Race Detectorに関する一般的な知識
• io/ioutilパッケージの機能に関する知識
• チャネルを用いたGoの並行処理に関する知識
• コミットメッセージに記載されているビルドログのURL (現在はアクセスできない可能性がありますが、当時の状況を理解する手がかりとなります)
• Go CL 87640043: https://golang.org/cl/87640043 (Gerrit上の変更リスト)