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

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

このコミットは、Go言語の標準ライブラリ net/http/httputil パッケージ内の DumpRequestOut 関数における競合状態(race condition)を修正するものです。具体的には、io.MultiWriter の引数の順序を変更することで、リクエストのダンプ処理が正しく行われるように改善しています。

コミット

commit f8d4bb884fdb7dc98ee9fb12a9847cb1f322ecbc
Author: Dave Cheney <dave@cheney.net>
Date:   Wed Aug 29 09:05:30 2012 +1000

    net/http/httputil: fix race in DumpRequestOut
    
    Fixes #3892.
    
    Swapping the order of the writers inside the MultiWriter ensures
    the request will be written to buf before http.ReadRequest completes.
    
    The fencedBuffer is not required to make the test pass on
    any machine that I have access too, but as the buf is shared
    across goroutines, I think it is necessary for correctness.
    
    R=bradfitz, fullung, franciscossouza
    CC=golang-dev
    https://golang.org/cl/6483061

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

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

元コミット内容

net/http/httputil: fix race in DumpRequestOut

DumpRequestOut 関数における競合状態を修正します。

MultiWriter 内のライターの順序を入れ替えることで、http.ReadRequest が完了する前にリクエストが buf に書き込まれることを保証します。

fencedBuffer は、私がアクセスできるどのマシンでもテストをパスさせるためには必要ありませんが、buf がゴルーチン間で共有されているため、正しさのためには必要だと考えます。

変更の背景

このコミットは、Go言語の標準ライブラリ net/http/httputil パッケージの DumpRequestOut 関数に存在していた競合状態を解決するために行われました。DumpRequestOut は、HTTPリクエストの内容をバイトスライスとしてダンプ(出力)するためのユーティリティ関数です。この関数は、内部的に http.Transport を利用してリクエストを「送信」し、その過程でリクエストの生データをキャプチャします。

問題は、リクエストの書き込み先として io.MultiWriter が使用されていた点にありました。io.MultiWriter は複数の io.Writer を結合し、1つの Write 呼び出しで全ての結合されたライターにデータを書き込む機能を提供します。DumpRequestOut の実装では、リクエストデータをキャプチャするためのバッファ (buf) と、リクエストの読み取り側 (dr) にデータを送るためのパイプの書き込み側 (pw) が MultiWriter に渡されていました。

元のコードでは、io.MultiWriter(pw, &buf) の順序でライターが指定されていました。これにより、pw への書き込みが &buf への書き込みよりも先に、あるいは並行して行われる可能性がありました。http.ReadRequestdr からデータを読み取りますが、もし pw への書き込みが &buf への書き込みよりも先行し、かつ http.ReadRequestbuf にデータが完全に書き込まれる前に読み取りを完了してしまうと、buf には不完全なリクエストデータしか含まれない、という競合状態が発生していました。これは、DumpRequestOut が期待する「完全なリクエストのダンプ」という目的を達成できないことを意味します。

コミットメッセージにある #3892 は、Goプロジェクトの内部的な課題追跡システムにおける問題番号である可能性が高いです。この問題は、DumpRequestOut が特定の条件下で不正確な出力を生成するというバグとして報告されたものと推測されます。

前提知識の解説

Go言語の並行処理と競合状態

Go言語はゴルーチン(goroutine)とチャネル(channel)を用いた並行処理を強力にサポートしています。ゴルーチンは軽量なスレッドのようなもので、Goランタイムによって管理されます。複数のゴルーチンが同時に実行されることで、プログラムの並行性が向上します。

しかし、複数のゴルーチンが共有リソース(この場合は buf)に同時にアクセスし、少なくとも1つのゴルーチンがそのリソースを変更する場合、競合状態(race condition)が発生する可能性があります。競合状態が発生すると、プログラムの実行順序によって結果が非決定論的になり、予期せぬバグ(この場合は不完全なリクエストダンプ)が発生します。

io.MultiWriter

io.MultiWriter は、Go言語の io パッケージで提供されるユーティリティ関数です。これは、複数の io.Writer インターフェースを実装するオブジェクトを受け取り、それらを1つの io.Writer として結合します。結合された WriterWrite メソッドが呼び出されると、引数として渡された全ての Writer に対して同じデータが書き込まれます。

MultiWriter の内部実装では、引数として渡されたライターの順序で Write メソッドが呼び出されます。つまり、io.MultiWriter(writerA, writerB) とした場合、writerA への書き込みが writerB への書き込みよりも先に試みられます。この順序が、今回の競合状態の修正において重要な役割を果たします。

net/http パッケージ

net/http パッケージは、HTTPクライアントとサーバーの実装を提供します。

  • http.Request: HTTPリクエストを表す構造体です。
  • http.Transport: HTTPクライアントがリクエストを送信し、レスポンスを受信する際の低レベルな詳細(コネクションの確立、プロキシの利用など)を制御する構造体です。Dial フィールドに関数を設定することで、コネクションの確立方法をカスタマイズできます。
  • http.ReadRequest: io.Reader からHTTPリクエストを読み取る関数です。

技術的詳細

DumpRequestOut 関数は、http.Request オブジェクトを受け取り、そのリクエストの生データをバイトスライスとして返します。この関数は、リクエストを実際にネットワークに送信する代わりに、カスタムの http.Transport を使用してリクエストデータをキャプチャします。

元の実装では、http.TransportDial フィールドに設定される匿名関数内で、io.MultiWriter(pw, &buf) が使用されていました。ここで、pwio.PipeWriter のインスタンスであり、dr (io.PipeReader) と対になっています。http.ReadRequestdr からリクエストデータを読み取ります。一方、&bufbytes.Buffer のインスタンスであり、ダンプされるリクエストデータを格納するためのバッファです。

競合状態は以下のシナリオで発生していました。

  1. DumpRequestOut が呼び出され、内部で http.Transport を介してリクエストが「送信」されます。
  2. http.TransportDial 関数が呼び出され、dumpConn が作成されます。この dumpConnio.MultiWriter(pw, &buf) を使用してデータを書き込みます。
  3. リクエストデータが MultiWriter に書き込まれます。この際、pw への書き込みが &buf への書き込みよりも先に、または並行して行われる可能性があります。
  4. 別のゴルーチンで http.ReadRequestdr からデータの読み取りを開始します。
  5. もし pw への書き込みが先行し、http.ReadRequest がリクエストの終端(例えば、ヘッダーの終端を示す空行)を検出して読み取りを完了してしまった場合、&buf への書き込みがまだ完了していない可能性があります。
  6. 結果として、DumpRequestOut が返すバイトスライス(buf の内容)は、完全なリクエストデータを含まない不正確なものとなる、という問題が発生していました。

このコミットによる修正は、io.MultiWriter の引数の順序を io.MultiWriter(&buf, pw) に変更することです。この変更により、MultiWriter は常に &buf への書き込みを pw への書き込みよりも先に試みるようになります。

この順序の変更がなぜ競合状態を解決するのかというと、http.ReadRequestdr からデータを読み取る前に、リクエストの完全なデータが buf に書き込まれることが保証されるためです。buf への書き込みが完了してから pw への書き込みが行われるため、http.ReadRequest がリクエストの終端を検出する際には、すでに buf には完全なデータが格納されている状態になります。これにより、DumpRequestOut が常に正確なリクエストダンプを返すことが保証されます。

コミットメッセージにある fencedBuffer についての言及は、おそらくこの修正を検討する過程で、より複雑な同期メカニズム(例えば、ミューテックスなどで保護されたバッファ)が必要かどうかを議論したことを示唆しています。しかし、最終的には MultiWriter の引数順序の変更だけで問題が解決できると判断されたようです。これは、MultiWriter の内部的な書き込み順序の保証が、この特定の競合状態に対して十分な同期メカニズムとして機能したことを意味します。

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

--- a/src/pkg/net/http/httputil/dump.go
+++ b/src/pkg/net/http/httputil/dump.go
@@ -89,7 +89,7 @@ func DumpRequestOut(req *http.Request, body bool) ([]byte, error) {
 
 	t := &http.Transport{
 		Dial: func(net, addr string) (net.Conn, error) {
-			return &dumpConn{io.MultiWriter(pw, &buf), dr}, nil
+			return &dumpConn{io.MultiWriter(&buf, pw), dr}, nil
 		},
 	}
 

コアとなるコードの解説

変更は src/pkg/net/http/httputil/dump.go ファイルの DumpRequestOut 関数内で行われています。

元のコードでは、http.TransportDial フィールドに設定された匿名関数内で、dumpConn 構造体の初期化時に io.MultiWriter(pw, &buf) が使用されていました。

修正後のコードでは、この部分が io.MultiWriter(&buf, pw) に変更されています。

  • pw: io.PipeWriter のインスタンス。このライターに書き込まれたデータは、対応する io.PipeReader (dr) から読み取られます。http.ReadRequest はこの dr からリクエストデータを読み取ります。
  • &buf: bytes.Buffer のインスタンス。このバッファに書き込まれたデータが、最終的に DumpRequestOut 関数の戻り値となります。

この変更により、MultiWriter はまず &buf にデータを書き込み、その後に pw にデータを書き込むようになります。これにより、http.ReadRequestdr からリクエストデータを読み取る前に、buf に完全なリクエストデータが格納されることが保証され、競合状態が解消されます。

関連リンク

参考にした情報源リンク

  • コミットメッセージの内容
  • Go言語の公式ドキュメント (pkg.go.dev)
  • 競合状態に関する一般的な知識
  • io.MultiWriter の動作に関する一般的な知識