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

このコミットは、Go言語の標準ライブラリ net/http/httputil パッケージ内の DumpRequestOut 関数における競合状態（race condition）を修正するものです。具体的には、src/pkg/net/http/httputil/dump.go ファイルが変更され、DumpRequestOut 関数がより堅牢になるように修正されました。

コミット

commit b2935330b03bd9c8c691b3d98ce416d9017ce656
Author: Brad Fitzpatrick <bradfitz@golang.org>
Date:   Wed Feb 1 15:10:14 2012 -0800

    net/http/httputil: fix race in DumpRequestOut
    
    Fixes #2715
    
    R=golang-dev, adg
    CC=golang-dev
    https://golang.org/cl/5614043

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

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

元コミット内容

net/http/httputil: fix race in DumpRequestOut

このコミットは、net/http/httputil パッケージの DumpRequestOut 関数に存在する競合状態を修正します。

このコミットは、Go言語のIssue #2715「net/http/httputil: TestDumpRequest failure」を修正するために行われました。このIssueは、httputil.TestDumpRequest が特に GOMAXPROCS の値が高い環境（例: 16）で実行された際に、デッドロックとして現れるテストの失敗を報告していました。これは DumpRequestOut 関数が内部的に http.Transport を使用してリクエストをダンプする際に、特定の条件下で競合状態が発生し、テストが不安定になることが原因でした。

DumpRequestOut は、http.Transport が実際にワイヤー上で送信するであろうリクエストの形式をダンプするために設計されています。以前の実装では、カスタムの net.Conn を使用してリクエストの書き込みをキャプチャしていましたが、この方法が特定の並行処理シナリオで問題を引き起こしていました。この修正は、より堅牢な方法でリクエストのダンプを行うことで、この競合状態を解消することを目的としています。

前提知識の解説

net/http/httputil パッケージ: Go言語の標準ライブラリの一部で、HTTPユーティリティ関数を提供します。これには、HTTPリクエストやレスポンスをダンプ（内容を文字列化）する機能などが含まれます。
DumpRequestOut 関数: net/http/httputil パッケージの関数で、http.Transport が実際に送信するであろう形式でHTTPリクエストをバイト列としてダンプします。これには、User-Agent のような標準的なヘッダーも含まれます。
競合状態 (Race Condition): 複数のゴルーチン（またはスレッド）が共有リソースに同時にアクセスし、そのアクセス順序によってプログラムの最終結果が変わってしまう状態を指します。予期せぬ動作やバグの原因となります。
io.Pipe(): Go言語の io パッケージで提供される関数で、メモリ内で接続された io.Reader と io.Writer のペアを作成します。io.Writer に書き込まれたデータは、対応する io.Reader から読み取ることができます。これは、ストリームデータをゴルーチン間で安全に受け渡すためによく使用されます。
bufio.NewReader(): bufio パッケージの関数で、指定された io.Reader をバッファリングされたリーダーにラップします。これにより、読み取り操作の効率が向上します。
http.Transport: net/http パッケージの構造体で、HTTPクライアントがHTTPリクエストを送信し、レスポンスを受信する際の低レベルな詳細（コネクションの確立、プロキシの使用、TLS設定など）を管理します。カスタムの Dial 関数を設定することで、ネットワーク接続の挙動をカスタマイズできます。
net.Conn インターフェース: net パッケージで定義されるインターフェースで、ネットワーク接続の一般的な振る舞いを抽象化します。Read、Write、Close などのメソッドを持ちます。
io.MultiWriter(): io パッケージの関数で、複数の io.Writer を結合し、単一の io.Writer を返します。この結合されたライターに書き込まれたデータは、元のすべてのライターに複製して書き込まれます。
delegateReader: このコミットで新しく導入されたカスタムの io.Reader 実装です。これは、チャネルを介して別の io.Reader を受け取り、そのリーダーに読み取り操作を委譲することで、非同期的なデータフローを可能にします。

技術的詳細

以前の DumpRequestOut の実装では、http.Transport の Dial 関数をオーバーライドし、カスタムの net.Conn 実装である dumpConn を返していました。この dumpConn は、書き込み操作を bytes.Buffer にリダイレクトすることで、リクエストのバイト列をキャプチャしていました。しかし、このアプローチは、特に並行処理のシナリオにおいて、http.Transport の内部的な動作と競合し、デッドロックを引き起こす可能性がありました。具体的には、dumpConn の Reader が常にダミーのレスポンスを返すように設定されていたため、リクエストの送信とレスポンスの受信のタイミングがずれると問題が発生しました。

新しい実装では、この問題を解決するために、より洗練されたパイプとゴルーチンベースのアプローチを採用しています。

io.Pipe() の利用: io.Pipe() を使用して、pr (PipeReader) と pw (PipeWriter) のペアが作成されます。http.Transport の Dial 関数で返される dumpConn の Writer は、io.MultiWriter(pw, &buf) となります。これにより、http.Transport がリクエストを書き込むと、そのデータは pw を通じて pr に流れ、同時に buf (最終的なダンプ結果を格納する bytes.Buffer) にも書き込まれます。
非同期的なリクエスト読み取りとダミーレスポンス: 新しいゴルーチンが起動され、このゴルーチン内で http.ReadRequest(bufio.NewReader(pr)) が呼び出されます。これは pr からリクエストデータを読み取り、HTTPリクエストとしてパースします。リクエストの読み取りが完了すると、このゴルーチンは delegateReader のチャネル dr.c にダミーのHTTPレスポンス（HTTP/1.1 204 No Content）を strings.NewReader として送信します。
delegateReader の導入: dumpConn の Reader は、新しく導入された delegateReader のインスタンス dr になります。delegateReader は、内部に io.Reader を保持し、そのリーダーがチャネル c を通じて設定されるまで読み取り操作をブロックします。これにより、http.Transport がレスポンスを読み取ろうとするまで、ダミーレスポンスが準備されるのを待つことができます。
競合状態の解消: この設計により、リクエストの書き込みとレスポンスの読み取りが非同期的に、かつ安全に処理されます。http.Transport はリクエストを pw に書き込み、そのデータは別のゴルーチンで読み取られます。リクエストの読み取りが完了した後にのみ、ダミーレスポンスが delegateReader を通じて http.Transport に提供されます。これにより、以前の競合状態が解消され、DumpRequestOut がより安定して動作するようになります。

この変更は、http.Transport の内部的な動作を模倣しつつ、実際のネットワーク通信を行わずにリクエストのダンプを安全に行うための巧妙な手法です。

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

--- a/src/pkg/net/http/httputil/dump.go
+++ b/src/pkg/net/http/httputil/dump.go
@@ -5,8 +5,8 @@
 package httputil
 
 import (
+\t\"bufio\"\
 \t\"bytes\"\
-\t\"errors\"\
 \t\"fmt\"\
 \t\"io\"\
 \t\"io/ioutil\"\
@@ -47,40 +47,59 @@ func (c *dumpConn) SetWriteDeadline(t time.Time) error { return nil }\n // DumpRequestOut is like DumpRequest but includes\n // headers that the standard http.Transport adds,\n // such as User-Agent.\n-func DumpRequestOut(req *http.Request, body bool) (dump []byte, err error) {\n+func DumpRequestOut(req *http.Request, body bool) ([]byte, error) {\n \tsave := req.Body\n \tif !body || req.Body == nil {\n \t\treq.Body = nil\n \t} else {\n+\t\tvar err error\n \t\tsave, req.Body, err = drainBody(req.Body)\n \t\tif err != nil {\n-\t\t\treturn\n+\t\t\treturn nil, err\n \t\t}\n \t}\n \n-\tvar b bytes.Buffer\n-\tdialed := false\n+\t// Use the actual Transport code to record what we would send\n+\t// on the wire, but not using TCP.  Use a Transport with a\n+\t// customer dialer that returns a fake net.Conn that waits\n+\t// for the full input (and recording it), and then responds\n+\t// with a dummy response.\n+\tvar buf bytes.Buffer // records the output\n+\tpr, pw := io.Pipe()\n+\tdr := &delegateReader{c: make(chan io.Reader)}\n+\t// Wait for the request before replying with a dummy response:\n+\tgo func() {\n+\t\thttp.ReadRequest(bufio.NewReader(pr))\n+\t\tdr.c <- strings.NewReader(\"HTTP/1.1 204 No Content\\r\\n\\r\\n\")\n+\t}()\n+\n \tt := &http.Transport{\n-\t\tDial: func(net, addr string) (c net.Conn, err error) {\n-\t\t\tif dialed {\n-\t\t\t\treturn nil, errors.New(\"unexpected second dial\")\n-\t\t\t}\n-\t\t\tc = &dumpConn{\n-\t\t\t\tWriter: &b,\n-\t\t\t\tReader: strings.NewReader(\"HTTP/1.1 500 Fake Error\\r\\n\\r\\n\"),\n-\t\t\t}\n-\t\t\treturn\n+\t\tDial: func(net, addr string) (net.Conn, error) {\n+\t\t\treturn &dumpConn{io.MultiWriter(pw, &buf), dr}, nil\n \t\t},\n \t}\n \n-\t_, err = t.RoundTrip(req)\n+\t_, err := t.RoundTrip(req)\n \n \treq.Body = save\n \tif err != nil {\n-\t\treturn\n+\t\treturn nil, err\n \t}\n-\tdump = b.Bytes()\n-\treturn\n+\treturn buf.Bytes(), nil\n+}\n+\n+// delegateReader is a reader that delegates to another reader,\n+// once it arrives on a channel.\n+type delegateReader struct {\n+\tc chan io.Reader\n+\tr io.Reader // nil until received from c\n+}\n+\n+func (r *delegateReader) Read(p []byte) (int, error) {\n+\tif r.r == nil {\n+\t\tr.r = <-r.c\n+\t}\n+\treturn r.r.Read(p)\n }

コアとなるコードの解説

`DumpRequestOut` 関数の変更点

戻り値の変更:
- 変更前: (dump []byte, err error)
- 変更後: ([]byte, error)
- これは機能的な変更ではなく、Goの慣習に合わせた記述の簡略化です。
bufio パッケージのインポート追加:
- http.ReadRequest を使用するために bufio がインポートされました。
bytes.Buffer と io.Pipe の導入:
- 変更前は単一の bytes.Buffer b を使用していましたが、変更後は buf という bytes.Buffer と、io.Pipe() で作成される pr (PipeReader) および pw (PipeWriter) が導入されました。
- buf は最終的にダンプされるリクエストのバイト列を記録するために使用されます。
- pr と pw は、http.Transport がリクエストを書き込むストリームと、そのリクエストを読み取るストリームを分離するために使用されます。
delegateReader の導入とゴルーチンによる非同期処理:
- 変更前は dialed フラグと固定のダミーレスポンス (HTTP/1.1 500 Fake Error) を返す dumpConn を使用していました。
- 変更後、delegateReader 型の dr が作成され、その内部チャネル c を介して io.Reader を受け取ります。
- 新しいゴルーチンが起動され、このゴルーチン内で http.ReadRequest(bufio.NewReader(pr)) が実行されます。これは pw に書き込まれたリクエストデータを pr から読み取り、HTTPリクエストとしてパースします。
- リクエストの読み取りが完了すると、ゴルーチンは dr.c チャネルに strings.NewReader("HTTP/1.1 204 No Content\\r\\n\\r\\n") を送信します。これにより、http.Transport がレスポンスを読み取ろうとした際に、このダミーレスポンスが提供されます。
http.Transport の Dial 関数の変更:
- 変更前は dumpConn の Writer を &b に、Reader を固定のダミーレスポンスに設定していました。
- 変更後、dumpConn の Writer は io.MultiWriter(pw, &buf) となり、Reader は dr (delegateReader) となりました。
- io.MultiWriter(pw, &buf) により、http.Transport がリクエストを書き込むと、そのデータは pw にも buf にも同時に書き込まれます。pw に書き込まれたデータは、前述のゴルーチンで pr から読み取られます。

`delegateReader` 構造体の追加

このコミットで新しく delegateReader という構造体が定義されました。
c chan io.Reader: io.Reader を受け取るためのチャネルです。
r io.Reader: 実際に読み取り操作を委譲する io.Reader です。チャネルから値が来るまでは nil です。
Read(p []byte) (int, error) メソッド:
- r が nil の場合、c チャネルから io.Reader を受け取るまでブロックします。
- r が設定されたら、その r の Read メソッドを呼び出し、読み取り操作を委譲します。

この一連の変更により、DumpRequestOut は http.Transport の内部的なリクエスト/レスポンスサイクルをより正確にシミュレートしつつ、競合状態を回避して安全にリクエストをダンプできるようになりました。

参考にした情報源リンク

https://github.com/golang/go/issues/2715
https://github.com/golang/go/issues/38352 (関連するが、異なる問題)
Go言語の net/http および io パッケージのドキュメント

comemo