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

このコミットは、Go言語の net/http パッケージにおける Transport のデッドロック問題を修正するものです。具体的には、アイドル状態のHTTPチャネルで予期せぬレスポンスが受信された場合に発生する可能性のある競合状態とデッドロックを解消することを目的としています。

コミット

commit d645adc3d0f077e0271004c1b07ef89b2fd36522
Author: Yoshiyuki Kanno <nekotaroh@gmail.com>
Date:   Wed Jan 25 15:00:39 2012 -0800

    net/http: fix Transport deadlock
    
    This patch intend to fix following issues.
    http://code.google.com/p/go/issues/detail?id=2616
    
    Fixes #2616.
    
    R=golang-dev, bradfitz, nekotaroh
    CC=golang-dev
    https://golang.org/cl/5532057

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

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

元コミット内容

このコミットは、net/http パッケージの Transport におけるデッドロックを修正することを目的としています。具体的には、Go issue 2616で報告された問題に対応しています。

変更の背景

この変更の背景には、Go言語の標準ライブラリである net/http パッケージの Transport コンポーネントにおけるデッドロックの存在がありました。Transport はHTTPクライアントがネットワーク接続を管理し、リクエストを送信し、レスポンスを受信する役割を担っています。特に、接続の再利用（コネクションプーリング）を行う際に、複数のゴルーチンが同時に同じ接続にアクセスしようとしたり、予期せぬデータが受信されたりする状況で、内部状態の不整合が発生し、デッドロックに至る可能性がありました。

Go issue 2616（http://code.google.com/p/go/issues/detail?id=2616）で報告されたこの問題は、特に高負荷時や、サーバーが予期せず接続を閉じたり、クライアントがリクエストを送信していないにもかかわらずデータを受信したりするようなエッジケースで顕在化しました。このデッドロックは、アプリケーション全体の応答性を低下させ、最悪の場合、サービス停止につながる可能性がありました。

このコミットは、persistConn（永続的な接続を表す構造体）の readLoop メソッドにおけるロジックを修正することで、このデッドロックを解消しようとしています。

前提知識の解説

このコミットを理解するためには、以下のGo言語およびネットワークプログラミングに関する前提知識が必要です。

• Go言語の並行処理: Go言語はゴルーチン（goroutine）とチャネル（channel）を用いた並行処理を特徴としています。ゴルーチンは軽量なスレッドのようなもので、チャネルはゴルーチン間の安全な通信手段を提供します。デッドロックは、複数のゴルーチンが互いにリソースの解放を待ち合うことで発生する状態です。
• net/http パッケージ: Go言語の標準ライブラリで、HTTPクライアントとサーバーの実装を提供します。
  • http.Client: HTTPリクエストを送信するためのクライアント。
  • http.Transport: http.Client の内部で実際にネットワーク接続を管理し、リクエストの送信やレスポンスの受信を行うコンポーネント。コネクションプーリング（既存の接続を再利用する仕組み）を担います。
  • persistConn: http.Transport の内部で、単一の永続的なHTTP接続（Keep-Alive接続など）を管理する構造体。この構造体が、リクエストの送信とレスポンスの受信を調整します。
  • readLoop: persistConn のメソッドの一つで、バックグラウンドで接続からデータを読み取り、レスポンスを処理するゴルーチン内で実行されます。
  • numExpectedResponses: persistConn の内部状態を示すフィールドで、現在この接続でいくつのレスポンスが期待されているかを示すカウンターです。
  • lk (sync.Mutex): persistConn の内部状態を保護するためのミューテックス（排他ロック）。複数のゴルーチンが同時に persistConn のフィールドにアクセスするのを防ぎ、競合状態を回避します。
• HTTP Keep-Alive: HTTP/1.1で導入された機能で、単一のTCP接続で複数のHTTPリクエスト/レスポンスをやり取りできるようにします。これにより、接続の確立・切断のオーバーヘッドを削減し、パフォーマンスを向上させます。http.Transport はこのKeep-Alive接続を積極的に利用します。
• デッドロック: 複数のプロセスやスレッド（この場合はゴルーチン）が、互いに相手が保持しているリソースの解放を待ち、結果としてどのプロセスも実行を継続できない状態。

技術的詳細

このコミットの核心は、net/http/transport.go 内の persistConn 構造体の readLoop メソッドの変更にあります。

元のコードでは、readLoop が接続からデータを読み取る際に、まず pc.br.Peek(1) で1バイトをピーク（読み進めずに内容を確認）し、その後に pc.expectingResponse() を呼び出して、現在レスポンスが期待されているかどうかを確認していました。pc.expectingResponse() メソッドは、pc.lk.Lock() と pc.lk.Unlock() を使用して pc.numExpectedResponses の値に安全にアクセスしていました。

問題は、pc.expectingResponse() がロックを取得し、その後に readLoop が pc.close() を呼び出す可能性がある点にありました。pc.close() もまた pc.lk.Lock() を取得しようとします。もし readLoop が pc.expectingResponse() を呼び出した直後（ロックを解放した後）に、別のゴルーチンが pc.close() を呼び出し、かつ readLoop が pc.close() を呼び出す前に pc.lk.Lock() を取得しようとすると、デッドロックが発生する可能性がありました。特に、readLoop が pc.expectingResponse() で numExpectedResponses が0であることを確認し、その後 pc.close() を呼び出すまでの間に、別のゴルーチンが pc.close() を呼び出すと、両者が pc.lk のロックを待ち合う状態になります。

このコミットでは、この競合状態を解消するために、readLoop のロジックが以下のように変更されました。

1. expectingResponse() メソッドの削除: persistConn から expectingResponse() メソッドが削除されました。これは、このメソッドが readLoop の内部でロックを二重に取得する可能性のあるパターンを誘発していたためです。
2. readLoop 内でのロックの取得と解放の調整:
   • readLoop の中で、pc.br.Peek(1) の後に、すぐに pc.lk.Lock() を取得するように変更されました。
   • ロックを取得した状態で pc.numExpectedResponses == 0 をチェックします。
   • もし numExpectedResponses が0であれば、それは予期せぬレスポンスが受信されたことを意味するため、pc.closeLocked() を呼び出して接続を閉じます。pc.closeLocked() は既にロックが取得されていることを前提とした内部ヘルパー関数です。
   • その後、pc.lk.Unlock() を呼び出してロックを解放し、readLoop を終了します。
   • これにより、numExpectedResponses のチェックと接続のクローズが単一のロック保護されたクリティカルセクション内で行われるようになり、競合状態が解消されます。
3. closeLocked() ヘルパー関数の導入: pc.close() メソッドは、まずロックを取得し、その後 pc.closeLocked() を呼び出すように変更されました。pc.closeLocked() は、ロックが既に取得されていることを前提として、接続を実際にクローズするロジック（pc.broken = true, pc.conn.Close(), pc.mutateHeaderFunc = nil）を実行します。これにより、readLoop からも安全に接続をクローズできるようになりました。
4. テストコードの変更: src/pkg/net/http/transport_test.go から、Go issue 2616に関連する「既知の壊れたテスト」を示すコメントアウトされたコードが削除されました。これは、このコミットによって問題が修正され、テストがパスするようになったことを示唆しています。

これらの変更により、readLoop が numExpectedResponses をチェックし、必要に応じて接続をクローズする際に、他のゴルーチンとの間でデッドロックが発生する可能性がなくなりました。

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

変更は主に以下の2つのファイルに集中しています。

• src/pkg/net/http/transport.go (18行変更: 10挿入, 8削除)
• src/pkg/net/http/transport_test.go (4行削除)

src/pkg/net/http/transport.go の変更点

--- a/src/pkg/net/http/transport.go
+++ b/src/pkg/net/http/transport.go
@@ -494,12 +494,6 @@ func (pc *persistConn) isBroken() bool {
 	return pc.broken
 }
 
-func (pc *persistConn) expectingResponse() bool {
-	pc.lk.Lock()
-	defer pc.lk.Unlock()
-	return pc.numExpectedResponses > 0
-}
-
 var remoteSideClosedFunc func(error) bool // or nil to use default
 
 func remoteSideClosed(err error) bool {
@@ -518,14 +512,18 @@ func (pc *persistConn) readLoop() {
 
 	for alive {
 		pb, err := pc.br.Peek(1)
-		if !pc.expectingResponse() {
+
+		pc.lk.Lock()
+		if pc.numExpectedResponses == 0 {
+			pc.closeLocked()
+			pc.lk.Unlock()
 			if len(pb) > 0 {
 				log.Printf("Unsolicited response received on idle HTTP channel starting with %q; err=%v",
 					string(pb), err)
 			}
-			pc.close()
 			return
 		}
+		pc.lk.Unlock()
 
 		rc := <-pc.reqch
 
@@ -649,6 +647,10 @@ func (pc *persistConn) roundTrip(req *transportRequest) (resp *Response, err err
 func (pc *persistConn) close() {
 	pc.lk.Lock()
 	defer pc.lk.Unlock()
+	pc.closeLocked()
+}
+
+func (pc *persistConn) closeLocked() {
 	pc.broken = true
 	pc.conn.Close()
 	pc.mutateHeaderFunc = nil

src/pkg/net/http/transport_test.go の変更点

--- a/src/pkg/net/http/transport_test.go
+++ b/src/pkg/net/http/transport_test.go
@@ -307,10 +307,6 @@ func TestTransportServerClosingUnexpectedly(t *testing.T) {
 // Test for http://golang.org/issue/2616 (appropriate issue number)
 // This fails pretty reliably with GOMAXPROCS=100 or something high.
 func TestStressSurpriseServerCloses(t *testing.T) {
-	if true {
-		t.Logf("known broken test; fix coming. Issue 2616")
-		return
-	}
 	if testing.Short() {
 		t.Logf("skipping test in short mode")
 		return

コアとなるコードの解説

src/pkg/net/http/transport.go

1. expectingResponse() メソッドの削除: func (pc *persistConn) expectingResponse() bool が完全に削除されました。この関数は pc.lk をロックしてから pc.numExpectedResponses をチェックしていましたが、readLoop 内でこの関数を呼び出すと、readLoop が既に pc.lk をロックしている可能性があるため、デッドロックのリスクがありました。

2. readLoop() メソッドの変更:

   • 変更前:

     if !pc.expectingResponse() {
         // ...
         pc.close()
         return
     }
     

     ここでは pc.expectingResponse() がロックを取得・解放し、その後 pc.close() が再度ロックを取得しようとしていました。この間に競合状態が発生する可能性がありました。
   • 変更後:

     pc.lk.Lock() // ここでロックを取得
     if pc.numExpectedResponses == 0 {
         pc.closeLocked() // ロックを保持したままクローズ処理
         pc.lk.Unlock()   // ロックを解放
         if len(pb) > 0 {
             log.Printf("Unsolicited response received on idle HTTP channel starting with %q; err=%v",
                 string(pb), err)
         }
         return
     }
     pc.lk.Unlock() // ロックを解放
     

     この変更により、numExpectedResponses のチェックと、それに続く接続のクローズ処理（pc.closeLocked()）が、単一のロック（pc.lk）によって保護されるようになりました。これにより、readLoop がアイドル状態の接続で予期せぬデータを受信した場合に、安全に接続を閉じることができるようになり、デッドロックが回避されます。

3. closeLocked() ヘルパー関数の導入:

   • 新しいプライベートメソッド func (pc *persistConn) closeLocked() が追加されました。このメソッドは、呼び出し元が既に pc.lk のロックを保持していることを前提として、接続を実際にクローズするロジック（pc.broken = true, pc.conn.Close(), pc.mutateHeaderFunc = nil）を実行します。
   • 既存の func (pc *persistConn) close() メソッドは、まず pc.lk.Lock() を取得し、defer pc.lk.Unlock() で解放を予約し、その後 pc.closeLocked() を呼び出すように変更されました。これにより、close() を呼び出す側はロックの状態を気にすることなく安全に接続を閉じることができ、readLoop のような内部ロジックからは、既にロックが取得されている状態で closeLocked() を直接呼び出すことで、デッドロックを回避しつつ効率的に処理を行えるようになりました。

src/pkg/net/http/transport_test.go

• TestStressSurpriseServerCloses テスト関数から、if true { t.Logf("known broken test; fix coming. Issue 2616"); return } という行が削除されました。これは、このテストが以前はGo issue 2616のデッドロックを再現するために意図的にスキップされていたが、今回の修正によって問題が解決され、テストが正常に実行されるようになったことを示しています。

これらの変更は、persistConn の内部状態（特に numExpectedResponses）へのアクセスと、接続のクローズ処理におけるロックの粒度とタイミングを最適化することで、並行処理における競合状態とデッドロックを根本的に解決しています。

関連リンク

• Go issue 2616: http://code.google.com/p/go/issues/detail?id=2616 (現在はGoのIssue TrackerがGitHubに移行しているため、直接アクセスできない可能性がありますが、当時の問題報告のIDです。)
• Go CL 5532057: https://golang.org/cl/5532057 (Gerrit Code Reviewへのリンク。当時のコードレビューの詳細を確認できます。)

参考にした情報源リンク

• Go issue 2616に関するWeb検索結果
• Go言語の net/http パッケージのドキュメント (当時のバージョン)
• Go言語の並行処理に関する一般的な知識 (ゴルーチン、チャネル、ミューテックス)
• HTTP/1.1 Keep-Aliveの仕組み
• デッドロックの概念と一般的な解決策