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

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

このコミットは、Go言語の標準ライブラリ net/http パッケージ内の transport.go ファイルにおける競合状態（race condition）を修正するものです。具体的には、persistConn 型の roundTrip メソッド内で、mutateHeaderFunc の呼び出しと numExpectedResponses のインクリメントに関する同期の問題を解決しています。これにより、HTTPリクエストの処理がより堅牢になります。

コミット

commit 1aa56a239634a31cc1812c37f0bbfa4378df7b9e
Author: Brad Fitzpatrick <bradfitz@golang.org>
Date:   Wed Jan 30 15:10:07 2013 -0800

    net/http: fix race
    
    Fixes #4724
    
    R=golang-dev, rsc
    CC=golang-dev
    https://golang.org/cl/7225071

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

https://github.com/golang/go/commit/1aa56a239634a31cc1812c37f0bbfa4378df7b9e

元コミット内容

このコミットの目的は、net/http パッケージにおける競合状態を修正することです。コミットメッセージには「Fixes #4724」とあり、これはGoのIssueトラッカーにおけるIssue 4724を修正したことを示しています。このIssueは、net/http クライアントが複数のリクエストを同時に処理する際に発生する可能性のある競合状態について報告していました。

変更の背景

Goの net/http パッケージは、HTTPクライアントとサーバーの実装を提供します。クライアント側では、http.Transport が実際のHTTPリクエストの送信とレスポンスの受信を担当します。この Transport は、パフォーマンス向上のためにTCP接続を再利用する「永続的な接続（persistent connection）」の概念を使用します。persistConn はこの永続的な接続を管理する内部構造体です。

問題となっていたのは、persistConn の roundTrip メソッド内で、mutateHeaderFunc の呼び出しと numExpectedResponses カウンタの更新が、適切に同期されていなかった点です。mutateHeaderFunc は、リクエストヘッダを送信直前に変更するためのオプションの関数です。numExpectedResponses は、この永続接続上で現在処理中の（期待される）レスポンスの数を追跡するためのカウンタです。

元のコードでは、mutateHeaderFunc の呼び出しがロックの外で行われていました。その一方で、numExpectedResponses のインクリメントはロックの内側で行われていました。もし mutateHeaderFunc が実行されている間に、別のゴルーチンが同じ persistConn に対して roundTrip を呼び出そうとした場合、mutateHeaderFunc が非同期に実行されることで、mutateHeaderFunc がアクセスする可能性のある persistConn のフィールド（例えば mutateHeaderFunc 自体）が、別のゴルーチンによって変更される可能性がありました。これにより、mutateHeaderFunc がnilポインタにアクセスしたり、予期しない動作を引き起こしたりする競合状態が発生する可能性がありました。

Issue #4724の報告によると、この競合状態は、特に Transport の DisableKeepAlives が true に設定されている場合に顕著に発生し、mutateHeaderFunc がnilになることでパニックを引き起こす可能性がありました。

前提知識の解説

競合状態 (Race Condition)

競合状態とは、複数のゴルーチン（またはスレッド）が共有リソース（メモリ、ファイルなど）に同時にアクセスし、そのアクセス順序によってプログラムの最終結果が変わってしまう状態を指します。これは、並行処理における一般的なバグの原因であり、デバッグが困難な場合があります。

sync.Mutex

Go言語の sync パッケージは、並行処理における同期プリミティブを提供します。sync.Mutex は相互排他ロック（mutual exclusion lock）を実装しており、共有リソースへのアクセスを一度に一つのゴルーチンに制限するために使用されます。

  • Lock(): ロックを取得します。既にロックが取得されている場合は、ロックが解放されるまでゴルーチンはブロックされます。
  • Unlock(): ロックを解放します。

net/http パッケージのクライアント側処理

net/http パッケージのクライアント側では、http.Client がHTTPリクエストを送信するためのエントリポイントです。Client は内部的に http.Transport を使用して実際のネットワーク通信を行います。

  • http.Transport: HTTPリクエストのラウンドトリップ（リクエストの送信からレスポンスの受信まで）を処理するインターフェースです。TCP接続の確立、接続の再利用（Keep-Alive）、プロキシの処理などを担当します。
  • persistConn: Transport が内部的に使用する構造体で、単一の永続的なTCP接続を表します。複数のHTTPリクエストが同じTCP接続を介して送受信されることを可能にし、接続確立のオーバーヘッドを削減します。
  • roundTrip メソッド: persistConn のメソッドで、単一のHTTPリクエストをこの永続接続上で送信し、対応するレスポンスを受信します。

mutateHeaderFunc

persistConn 構造体内の mutateHeaderFunc フィールドは、func(Header) 型の関数ポインタです。これは、HTTPリクエストが実際にネットワークに書き込まれる直前に、そのリクエストのヘッダを修正するために使用されるオプションのコールバック関数です。例えば、特定の認証ヘッダを追加したり、ユーザーエージェントを変更したりするのに使われます。

技術的詳細

このコミットが修正する競合状態は、persistConn.roundTrip メソッド内で発生していました。元のコードは以下のようになっていました（関連部分のみ抜粋）：

type persistConn struct {
    // ...
    mutateHeaderFunc func(Header)

    lk                   sync.Mutex // guards numExpectedResponses and broken
    numExpectedResponses int
    broken               bool // an error has happened on this connection; marked broken so it's not reused.
}

func (pc *persistConn) roundTrip(req *transportRequest) (resp *Response, err error) {
    if pc.mutateHeaderFunc != nil {
        pc.mutateHeaderFunc(req.extraHeaders()) // (A) ロックの外でmutateHeaderFuncを呼び出し
    }

    // ...

    pc.lk.Lock()
    pc.numExpectedResponses++ // (B) ロックの中でnumExpectedResponsesをインクリメント
    pc.lk.Unlock()

    // ...
}

このコードのフローには以下の問題がありました：

  1. mutateHeaderFunc の競合: mutateHeaderFunc フィールド自体は lk ミューテックスによって保護されていませんでした。もしゴルーチンAが roundTrip を実行し、(A)の行で pc.mutateHeaderFunc を読み取った直後に、別のゴルーチンBが persistConn を変更し、pc.mutateHeaderFunc をnilに設定したとします。すると、ゴルーチンAが pc.mutateHeaderFunc(req.extraHeaders()) を呼び出そうとした際に、nilポインタデリファレンスが発生し、パニックを引き起こす可能性がありました。

  2. numExpectedResponses のインクリメントタイミング: numExpectedResponses のインクリメントはロックの内側で行われていましたが、mutateHeaderFunc の呼び出しはロックの外で行われていました。これは、mutateHeaderFunc が実行される時点では、numExpectedResponses がまだインクリメントされていないことを意味します。もし mutateHeaderFunc が numExpectedResponses の値に依存するようなロジックを含んでいた場合、不正確な値に基づいて動作する可能性がありました（ただし、この特定のケースでは mutateHeaderFunc は numExpectedResponses に直接依存していませんでした）。より重要なのは、mutateHeaderFunc の呼び出し自体が persistConn の状態に依存する可能性があり、その状態がロックによって保護されていないことでした。

このコミットは、これらの問題を解決するために、mutateHeaderFunc の読み取りと numExpectedResponses のインクリメントを、lk ミューテックスによって保護されたクリティカルセクション内に移動させました。

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

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

  1. persistConn 構造体のフィールド順序変更:

    --- a/src/pkg/net/http/transport.go
+++ b/src/pkg/net/http/transport.go
@@ -529,14 +529,13 @@ type persistConn struct {
  	closech  chan struct{}       // broadcast close when readLoop (TCP connection) closes
  	isProxy  bool
  
+\tlk                   sync.Mutex // guards following 3 fields
+\tnumExpectedResponses int
+\tbroken               bool // an error has happened on this connection; marked broken so it's not reused.
  	// mutateHeaderFunc is an optional func to modify extra
  	// headers on each outbound request before it's written. (the
  	// original Request given to RoundTrip is not modified)
  	mutateHeaderFunc func(Header)
-
-\tlk                   sync.Mutex // guards numExpectedResponses and broken
-\tnumExpectedResponses int
-\tbroken               bool // an error has happened on this connection; marked broken so it's not reused.
 }

    lk, numExpectedResponses, broken の3つのフィールドの宣言が、mutateHeaderFunc の上（より正確には、mutateHeaderFunc のコメントブロックの上）に移動されました。これは機能的な変更ではなく、コードの可読性と論理的なグループ化を改善するためのものです。lk がその後の3つのフィールド（numExpectedResponses, broken, mutateHeaderFunc）を保護することを示すコメントが追加されています。

  2. roundTrip メソッド内のロックと処理順序の変更:

    --- a/src/pkg/net/http/transport.go
+++ b/src/pkg/net/http/transport.go
@@ -711,8 +710,13 @@ type writeRequest struct {\n }\n \n func (pc *persistConn) roundTrip(req *transportRequest) (resp *Response, err error) {\n-\tif pc.mutateHeaderFunc != nil {\n-\t\tpc.mutateHeaderFunc(req.extraHeaders())\n+\tpc.lk.Lock()\n+\tpc.numExpectedResponses++\n+\theaderFn := pc.mutateHeaderFunc\n+\tpc.lk.Unlock()\n+\n+\tif headerFn != nil {\n+\t\theaderFn(req.extraHeaders())\n \t}\n \n  \t// Ask for a compressed version if the caller didn\'t set their\n@@ -728,10 +732,6 @@ func (pc *persistConn) roundTrip(req *transportRequest) (resp *Response, err err\n  \t\treq.extraHeaders().Set(\"Accept-Encoding\", \"gzip\")\n  \t}\n  \n-\tpc.lk.Lock()\n-\tpc.numExpectedResponses++\n-\tpc.lk.Unlock()\n-\n  \t// Write the request concurrently with waiting for a response,\n  \t// in case the server decides to reply before reading our full\n  \t// request body.\n    ```
これが主要な変更点です。
-   `pc.lk.Lock()` が `roundTrip` メソッドの冒頭に移動されました。
-   `pc.numExpectedResponses++` がロック直後に行われるようになりました。
-   `pc.mutateHeaderFunc` の値が、ロックが保持されている間にローカル変数 `headerFn` にコピーされるようになりました (`headerFn := pc.mutateHeaderFunc`)。
-   その後、`pc.lk.Unlock()` が呼び出され、ロックが解放されます。
-   最後に、ローカル変数 `headerFn` を使用して `headerFn(req.extraHeaders())` が呼び出されます。これにより、`mutateHeaderFunc` の実際の呼び出しはロックの外で行われますが、`mutateHeaderFunc` の値自体はロックによって保護された状態で安全に取得されています。
-   元のコードの後半にあった `pc.lk.Lock()` と `pc.numExpectedResponses++` のブロックは削除されました。

コアとなるコードの解説

この変更の核心は、mutateHeaderFunc の値の読み取りと numExpectedResponses のインクリメントを、単一の原子的な操作として lk ミューテックスによって保護することです。

  1. pc.lk.Lock() と pc.numExpectedResponses++ の移動: roundTrip メソッドの開始時にロックを取得し、すぐに numExpectedResponses をインクリメントすることで、このカウンタの更新がリクエスト処理の非常に早い段階で、かつ安全に行われることが保証されます。これにより、この接続上で新しいリクエストが処理中であることが即座に反映されます。

  2. headerFn := pc.mutateHeaderFunc: 最も重要な変更は、pc.mutateHeaderFunc の値をロックが保持されている間にローカル変数 headerFn にコピーする点です。これにより、pc.mutateHeaderFunc フィールドが他のゴルーチンによって変更されたとしても、この roundTrip 呼び出し内で使用される mutateHeaderFunc の値は、ロックが取得された時点での正しい値が保証されます。ロックを解放した後に headerFn を呼び出すことで、mutateHeaderFunc の実行自体がロックを長時間保持することなく行われ、並行性を損なうことなく安全性が確保されます。

この修正により、mutateHeaderFunc がnilになることによるパニックや、persistConn の状態が予期せず変更されることによる競合状態が解消されます。numExpectedResponses のインクリメントも、リクエスト処理の開始時に適切に同期されるようになりました。

関連リンク

  • Go Issue 4724: https://github.com/golang/go/issues/4724
  • Gerrit Change-Id: I2222222222222222222222222222222222222222 (コミットメッセージの https://golang.org/cl/7225071 に対応するGerritのChange-Id)

参考にした情報源リンク

  • Go Issue 4724の議論: 上記のIssueリンクを参照。
  • Go言語の sync パッケージドキュメント: https://pkg.go.dev/sync
  • Go言語の net/http パッケージドキュメント: https://pkg.go.dev/net/http
  • Go言語における競合状態と同期プリミティブに関する一般的な情報源。