Keyboard shortcuts

Press or to navigate between chapters

Press S or / to search in the book

Press ? to show this help

Press Esc to hide this help

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

このコミットは、Go言語の標準ライブラリである net/http パッケージ内のテストファイル src/pkg/net/http/transport_test.go における、TestTransportReading100Continue というテストの競合状態(race condition)を修正するものです。具体的には、テスト内でネットワーク接続をシミュレートするために使用されていた io.Pipe の設定が不適切であった点を改善し、より現実的な双方向通信を模倣するように変更しています。また、不要になったパイプ登録ロジックの削除も行われています。

コミット

commit ad7aa8302011f08c2cac5291697704b352c2b735
Author: Brad Fitzpatrick <bradfitz@golang.org>
Date:   Sat Mar 30 11:18:56 2013 -0700

    net/http: fix incredibly racy TestTransportReading100Continue
    
    Whoops. I'm surprised it even worked before. (Need two pipes,
    not one.)
    
    Also, remove the whole pipe registration business, since it
    wasn't even required in the previous version. (I'd later fixed
    it at the end of send100Response, but forgot to delete it)
    
    R=golang-dev, r
    CC=golang-dev
    https://golang.org/cl/8191044

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

https://github.com/golang/go/commit/ad7aa8302011f08c2cac5291697704b352c2b735

元コミット内容

このコミットは、net/http パッケージの TestTransportReading100Continue テストにおける深刻な競合状態を修正します。以前の実装では、ネットワーク接続のシミュレーションに単一の io.Pipe を使用していましたが、これはクライアントとサーバー間の双方向通信を正確に表現していませんでした。この不適切な設定が原因で、テストが不安定になり、予期せぬ動作を引き起こしていました。

また、以前のバージョンでは不要になっていたパイプ登録のロジック(registerPipe 関数と関連する writers 構造体)が残っていたため、これも削除されました。これは、send100Response 関数内で既に修正が行われていたにもかかわらず、その後のクリーンアップが忘れられていたためです。

変更の背景

net/http パッケージは、Go言語におけるHTTPクライアントおよびサーバーの実装を提供する重要な標準ライブラリです。その信頼性を保証するためには、厳密なテストが不可欠です。TestTransportReading100Continue は、HTTP/1.1 の 100 Continue ステータスコードの挙動をテストするためのものです。

100 Continue は、クライアントが大きなリクエストボディを送信する前に、サーバーがそのリクエストを受け入れる準備ができているかを確認するために使用される情報提供のステータスコードです。クライアントは Expect: 100-continue ヘッダを付けてリクエストを送信し、サーバーが 100 Continue を返せばボディの送信を続行し、そうでなければエラーとして処理します。

このテストでは、実際のネットワーク接続ではなく、io.Pipe を用いて仮想的な接続をシミュレートしていました。しかし、単一の io.Pipe では、クライアントからサーバーへのデータフローと、サーバーからクライアントへのデータフローという、TCP接続が持つ2つの独立した通信チャネルを正確に表現できませんでした。これにより、クライアントとサーバーが同時にパイプに書き込もうとしたり、読み取ろうとしたりする際に、データの順序が保証されず、競合状態が発生し、テストが不安定になっていました。

このコミットは、このテストの信頼性を向上させ、net/http パッケージの堅牢性を確保するために行われました。

前提知識の解説

  1. HTTP/1.1 100 Continue ステータスコード: HTTP/1.1 では、クライアントが大きなリクエストボディ(例: ファイルアップロード)を送信する際に、まずヘッダのみを送信し、サーバーがそのリクエストを受け入れるかどうかを確認するメカニズムがあります。クライアントは Expect: 100-continue ヘッダをリクエストに含めます。サーバーがこのヘッダを受け取り、リクエストの処理を続行できると判断した場合、100 Continue という情報提供のステータスコードを返します。これを受け取ったクライアントは、残りのリクエストボディを送信します。もしサーバーが 100 Continue 以外のエラーコード(例: 401 Unauthorized)を返した場合、クライアントはボディの送信を中止し、無駄なデータ転送を防ぐことができます。

  2. Go言語の io.Pipe: io.Pipe() は、Go言語の io パッケージで提供される関数で、メモリ上に同期的なパイプを作成します。これは io.Readerio.Writer のペアを返します。

    • io.PipeReader (pr): パイプの読み取り側。
    • io.PipeWriter (pw): パイプの書き込み側。 pw に書き込まれたデータは、pr から読み取ることができます。これは、異なるGoroutine間でデータをストリームとして受け渡す際に便利ですが、本質的には単一のデータフローを表します。TCPソケットのような双方向通信をシミュレートするには、工夫が必要です。
  3. TCP接続の双方向性: TCP (Transmission Control Protocol) は、インターネット上で信頼性の高いデータ転送を提供するプロトコルです。TCP接続は全二重(full-duplex)であり、これはデータが同時に両方向に流れることができることを意味します。つまり、クライアントはサーバーにデータを送信しながら、同時にサーバーからデータを受信することができます。これは、単一のパイプでは直接的に表現できない特性です。

  4. 競合状態 (Race Condition): 複数の並行プロセスやGoroutineが共有リソース(この場合は io.Pipe)にアクセスし、そのアクセス順序によって結果が非決定的に変わってしまう状態を指します。テストにおいて競合状態が発生すると、テストが時々成功したり失敗したりする「flaky test」となり、信頼性が損なわれます。

技術的詳細

このコミットの核心は、io.Pipe の使用方法を修正することで、HTTPクライアントとサーバー間の双方向通信をより正確にシミュレートすることにあります。

変更前: 変更前は、TestTransportReading100Continue テスト内で、ネットワーク接続のシミュレーションに単一の io.Pipe() が使用されていました。

pr, pw := io.Pipe()
conn := &rwTestConn{
    Reader: pr,
    Writer: pw,
}
go send100Response(pw, pr)

この設定では、conn.Writer (クライアントがサーバーにデータを送信する側) と send100Response (サーバーがクライアントにデータを送信する側) の両方が同じ pw (PipeWriter) に書き込もうとしていました。また、conn.Reader (クライアントがサーバーからデータを受信する側) と send100Response (サーバーがクライアントからデータを受信する側) の両方が同じ pr (PipeReader) から読み取ろうとしていました。

これは、実際のTCP接続の動作とは異なります。TCP接続では、クライアントからサーバーへのデータストリームと、サーバーからクライアントへのデータストリームは論理的に独立しています。単一の io.Pipe では、これらの独立したストリームを表現できず、クライアントとサーバーの読み書きが同じパイプを共有するため、データの順序が保証されず、競合状態が発生していました。例えば、クライアントがリクエストボディを書き込んでいる最中に、サーバーが 100 Continue レスポンスを同じパイプに書き込むと、データが混ざり合ったり、予期せぬ順序で読み取られたりする可能性がありました。

変更後: 変更後では、この問題を解決するために、2つの独立した io.Pipe() インスタンスが導入されました。

sr, sw := io.Pipe() // server read/write (サーバー視点での読み書き)
cr, cw := io.Pipe() // client read/write (クライアント視点での読み書き)

conn := &rwTestConn{
    Reader: cr, // クライアントは cr から読み取る (サーバーが cw に書き込む)
    Writer: sw, // クライアントは sw に書き込む (サーバーが sr から読み取る)
}
go send100Response(cw, sr) // サーバーは cw に書き込み、sr から読み取る

この新しい設定は、TCP接続の双方向性を正確に模倣しています。

  1. クライアントからサーバーへの通信パス:

    • クライアントは conn.Writer である sw にデータを書き込みます。
    • サーバーは send100Response 内で sr からデータを読み取ります。
    • swsr は1つの io.Pipe ペアを形成し、クライアントからサーバーへの一方向のデータフローを表現します。
  2. サーバーからクライアントへの通信パス:

    • サーバーは send100Response 内で cw にデータを書き込みます。
    • クライアントは conn.Reader である cr からデータを読み取ります。
    • cwcr は別の io.Pipe ペアを形成し、サーバーからクライアントへの一方向のデータフローを表現します。

このように2つの独立したパイプを使用することで、クライアントとサーバー間の読み書きが互いに干渉することなく、並行して行われるようになり、競合状態が解消されました。

また、以前のコミットで send100Response 関数内でパイプのクリーンアップが修正されたため、テスト開始時にパイプを登録して後で閉じるという writers 構造体と registerPipe 関数のロジックは不要になっていました。このコミットでは、その不要なコードも削除され、テストコードがより簡潔になりました。

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

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

--- a/src/pkg/net/http/transport_test.go
+++ b/src/pkg/net/http/transport_test.go
@@ -1404,23 +1404,6 @@ func TestTransportSocketLateBinding(t *testing.T) {
 func TestTransportReading100Continue(t *testing.T) {
 	defer afterTest(t)
 
-	var writers struct {
-		sync.Mutex
-		list []*io.PipeWriter
-	}
-	registerPipe := func(pw *io.PipeWriter) {
-		writers.Lock()
-		defer writers.Unlock()
-		writers.list = append(writers.list, pw)
-	}
-	defer func() {
-		writers.Lock()
-		defer writers.Unlock()
-		for _, pw := range writers.list {
-			pw.Close()
-		}
-	}()
-
 	const numReqs = 5
 	reqBody := func(n int) string { return fmt.Sprintf("request body %d", n) }
 	reqID := func(n int) string { return fmt.Sprintf("REQ-ID-%d", n) }
@@ -1463,13 +1446,13 @@ Content-Length: %d
 
 	tr := &Transport{
 		Dial: func(n, addr string) (net.Conn, error) {
-			pr, pw := io.Pipe()
-			registerPipe(pw)
+			sr, sw := io.Pipe() // server read/write
+			cr, cw := io.Pipe() // client read/write
 			conn := &rwTestConn{
-				Reader: pr,
-				Writer: pw,
+				Reader: cr,
+				Writer: sw,
 			}
-			go send100Response(pw, pr)
+			go send100Response(cw, sr)
 			return conn, nil
 		},
 		DisableKeepAlives: false,

コアとなるコードの解説

  1. 不要なパイプ登録ロジックの削除: TestTransportReading100Continue 関数の冒頭にあった writers 構造体と registerPipe 関数、およびそれらに関連する defer 句が完全に削除されました。これは、以前のコミットで send100Response 関数内のロジックが修正され、これらのパイプを明示的に管理する必要がなくなったためです。これにより、コードが簡潔になり、テストのオーバーヘッドが減少します。

  2. Dial 関数の io.Pipe の変更: Transport 構造体の Dial フィールドは、テスト目的でカスタムのネットワーク接続をシミュレートするために使用されます。この無名関数内で、接続の確立方法が変更されました。

    • 変更前:

      pr, pw := io.Pipe()
      registerPipe(pw) // 削除された行
      conn := &rwTestConn{
          Reader: pr,
          Writer: pw,
      }
      go send100Response(pw, pr)
      

      ここでは単一の io.Pipe が作成され、その読み取り側 (pr) と書き込み側 (pw) が rwTestConn に渡され、さらに send100Response にも渡されていました。これにより、クライアントとサーバーの通信が同じパイプを共有し、競合状態を引き起こしていました。

    • 変更後:

      sr, sw := io.Pipe() // サーバー側の読み書きパイプ
      cr, cw := io.Pipe() // クライアント側の読み書きパイプ
      conn := &rwTestConn{
          Reader: cr, // クライアントは cr から読み取る (サーバーが cw に書き込む)
          Writer: sw, // クライアントは sw に書き込む (サーバーが sr から読み取る)
      }
      go send100Response(cw, sr) // サーバーは cw に書き込み、sr から読み取る
      

      この変更により、2つの独立した io.Pipe ペアが作成されます。

      • sr, sw: サーバーがクライアントから読み取る (sr)、およびクライアントがサーバーに書き込む (sw) ためのパイプ。
      • cr, cw: クライアントがサーバーから読み取る (cr)、およびサーバーがクライアントに書き込む (cw) ためのパイプ。

      rwTestConn はクライアント側の視点での接続をシミュレートするため、その Readercr (サーバーからのデータを受信)、Writersw (サーバーへデータを送信) に設定されます。 send100Response はサーバー側のロジックをシミュレートするため、cw (クライアントへデータを送信) と sr (クライアントからデータを受信) が渡されます。

      この設定により、クライアントとサーバー間の双方向通信が、それぞれ独立したパイプを通じて行われるようになり、以前の競合状態が解消され、テストの信頼性が大幅に向上しました。

関連リンク

参考にした情報源リンク

  • Go言語の公式ドキュメント
  • HTTP/1.1 の仕様に関するRFC
  • Go言語のソースコード (特に net/http パッケージのテストコード)
  • 競合状態に関する一般的なプログラミングの概念