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

このコミットは、Go言語の net/http パッケージにおける bodyEOFSignal.isClosed フィールドに関する競合状態（race condition）を修正するものです。具体的には、HTTPレスポンスボディの読み込み終了を通知するメカニズムにおいて発生しうる、未報告の競合状態によるパニック（panic）を解消します。

コミット

• コミットハッシュ: f1b1753627d1f895baa53e41c6fb4301282a3760
• Author: Dave Cheney dave@cheney.net
• Date: Fri Oct 12 08:32:56 2012 +1100

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

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

元コミット内容

net/http: fix race on bodyEOFSignal.isClosed

Update #4191.

Fixes unreported race failure at
http://build.golang.org/log/61e43a328fb220801d3d5c88cd91916cfc5dc43c

R=dvyukov, bradfitz
CC=golang-dev
https://golang.org/cl/6640057

変更の背景

このコミットは、Go言語の標準ライブラリである net/http パッケージにおいて、HTTPクライアントがレスポンスボディを処理する際に発生する可能性のある競合状態を修正するために行われました。具体的には、bodyEOFSignal という内部構造体内の isClosed フィールドが複数のゴルーチンから同時にアクセスされることで、データ競合が発生し、プログラムがパニックに陥る可能性がありました。

この問題は、Goのビルドシステムで検出された未報告の競合状態の失敗（http://build.golang.org/log/61e43a328fb220801d3d5c88cd91916cfc5dc43c）として顕在化しました。また、関連するIssueとして Issue #4191: net/http: Unrecoverable client panic crashes my program が挙げられています。このIssueは、net/http クライアントが回復不能なパニックを引き起こし、プログラム全体がクラッシュするという報告でした。

bodyEOFSignal は、HTTPレスポンスボディの読み込みが終了したことを通知するためのメカニズムです。isClosed フィールドは、このシグナルが既にクローズされたかどうかを示すフラグとして機能していました。しかし、このフラグがアトミックに（不可分な操作として）扱われていなかったため、複数のゴルーチンが同時に isClosed の値を読み書きしようとすると、予期せぬ動作やパニックが発生する可能性がありました。

前提知識の解説

競合状態 (Race Condition)

競合状態とは、複数の並行プロセスやスレッド（Goにおいてはゴルーチン）が共有リソース（メモリ上の変数など）にアクセスする際に、そのアクセス順序によって結果が非決定的に変わってしまう状態を指します。特に、読み取りと書き込みが同時に行われる場合に問題となりやすく、データの破損やプログラムのクラッシュを引き起こす可能性があります。

sync/atomic パッケージ

Go言語の sync/atomic パッケージは、低レベルのアトミック操作を提供します。アトミック操作とは、その操作が中断されることなく完全に実行されることを保証するものです。これにより、複数のゴルーチンが同時に共有変数にアクセスしても、競合状態を回避し、データの整合性を保つことができます。atomic パッケージは、ミューテックス（sync.Mutex）のようなロック機構よりも粒度が細かく、特定の操作（読み書き、比較交換など）に特化しているため、パフォーマンスが重要な場面で利用されます。

• atomic.LoadUint32(&addr): addr に格納されている uint32 の値をアトミックに読み込みます。
• atomic.StoreUint32(&addr, val): addr に val をアトミックに書き込みます。
• atomic.CompareAndSwapUint32(&addr, old, new): addr の値が old と等しい場合にのみ、new を addr にアトミックに書き込みます。成功した場合は true を、失敗した場合は false を返します。

sync.Once

sync.Once は、Go言語で提供されるユーティリティで、特定の関数が一度だけ実行されることを保証します。複数のゴルーチンが同時に Do メソッドを呼び出しても、内部の関数は一度しか実行されません。これは、初期化処理など、一度だけ実行されるべき処理に非常に便利です。

io.ReadCloser インターフェース

io.ReadCloser は、Go言語の標準ライブラリ io パッケージで定義されているインターフェースです。これは io.Reader と io.Closer の両方のインターフェースを組み合わせたものです。

• io.Reader: Read(p []byte) (n int, err error) メソッドを持ち、データを読み込む機能を提供します。
• io.Closer: Close() error メソッドを持ち、リソースをクローズする機能を提供します。

HTTPレスポンスボディは通常、io.ReadCloser として扱われ、読み込みとクローズの両方が可能です。

panic と recover

Go言語における panic は、プログラムの実行を中断させるエラーの一種です。通常、回復不能なエラーやプログラマの論理的な誤りを示すために使用されます。panic が発生すると、現在のゴルーチンの実行が停止し、遅延関数（defer）が実行され、コールスタックを遡っていきます。

recover は、panic から回復するための組み込み関数です。recover は defer 関数内でしか呼び出すことができず、panic が発生した際に recover を呼び出すと、panic の引数（通常はエラー値）が返され、プログラムの実行を継続できます。このコミットで修正された問題は、panic を引き起こす競合状態でした。

技術的詳細

このコミットの核心は、net/http パッケージ内の bodyEOFSignal 構造体における isClosed フィールドの扱いを変更することです。

元の実装では、isClosed は単純な bool 型でした。

type bodyEOFSignal struct {
    body     io.ReadCloser
    fn       func()
    isClosed bool
    once     sync.Once
}

この bool 型のフィールドは、複数のゴルーチンから同時に読み書きされる可能性がありました。例えば、Read メソッドが isClosed を読み取っている最中に、別のゴルーチンが Close メソッドを呼び出して isClosed を書き換えるといった状況です。このような非同期なアクセスは、競合状態を引き起こし、予期せぬ panic を発生させる原因となっていました。具体的には、Read メソッド内で es.isClosed && n > 0 のチェックが行われた後に isClosed が変更され、その後の処理で panic("http: unexpected bodyEOFSignal Read after Close; see issue 1725") が発生する可能性がありました。

この問題を解決するために、isClosed フィールドは uint32 型に変更され、sync/atomic パッケージのアトミック操作を用いてアクセスされるようになりました。

type bodyEOFSignal struct {
    body     io.ReadCloser
    fn       func()
    isClosed uint32 // atomic bool, non-zero if true
    once     sync.Once
}

uint32 を使用し、0 を false、1 を true として扱うことで、atomic.LoadUint32 や atomic.CompareAndSwapUint32 といったアトミックな読み書き操作が可能になります。これにより、isClosed の値の読み書きが不可分な操作となり、複数のゴルーチンからの同時アクセスによる競合状態が解消されます。

具体的には、以下の2つのヘルパーメソッドが導入されました。

• closed() bool: isClosed の値をアトミックに読み取り、クローズされているかどうかを返します。
• setClosed() bool: isClosed の値をアトミックに true に設定しようとします。既に true であった場合は false を返し、false から true に変更できた場合は true を返します。これにより、Close メソッドが複数回呼び出されても、実際のクローズ処理は一度だけ実行されることが保証されます。

これらの変更により、bodyEOFSignal の状態管理がスレッドセーフになり、競合状態によるパニックが防止されます。

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

--- a/src/pkg/net/http/transport.go
+++ b/src/pkg/net/http/transport.go
@@ -24,6 +24,7 @@ import (
 	"os"
 	"strings"
 	"sync"
+	"sync/atomic"
 	"time"
 )
 
@@ -816,13 +817,13 @@ func responseIsKeepAlive(res *Response) bool {
 type bodyEOFSignal struct {
 	body     io.ReadCloser
 	fn       func()
-	isClosed bool
+	isClosed uint32 // atomic bool, non-zero if true
 	once     sync.Once
 }
 
 func (es *bodyEOFSignal) Read(p []byte) (n int, err error) {
 	n, err = es.body.Read(p)
-	if es.isClosed && n > 0 {
+	if es.closed() && n > 0 {
 		panic("http: unexpected bodyEOFSignal Read after Close; see issue 1725")
 	}
 	if err == io.EOF {
@@ -832,10 +833,10 @@ func (es *bodyEOFSignal) Read(p []byte) (n int, err error) {
 }
 
 func (es *bodyEOFSignal) Close() (err error) {
-	if es.isClosed {
+	if !es.setClosed() {
+		// already closed
 		return nil
 	}
-	es.isClosed = true
 	err = es.body.Close()
 	if err == nil {
 		es.condfn()
@@ -849,6 +850,14 @@ func (es *bodyEOFSignal) condfn() {
 	}
 }
 
+func (es *bodyEOFSignal) closed() bool {
+	return atomic.LoadUint32(&es.isClosed) != 0
+}
+
+func (es *bodyEOFSignal) setClosed() bool {
+	return atomic.CompareAndSwapUint32(&es.isClosed, 0, 1)
+}
+
 type readFirstCloseBoth struct {
 	io.ReadCloser
 	io.Closer

コアとなるコードの解説

src/pkg/net/http/transport.go

1. import "sync/atomic" の追加: bodyEOFSignal の isClosed フィールドをアトミックに操作するために、sync/atomic パッケージがインポートされました。

2. bodyEOFSignal 構造体の変更:

   • isClosed bool が isClosed uint32 // atomic bool, non-zero if true に変更されました。これにより、isClosed がアトミック操作の対象となる uint32 型の変数として扱われることが明示されます。コメントで atomic bool, non-zero if true とあるように、0 以外の値が true を意味します。

3. Read メソッドの変更:

   • if es.isClosed && n > 0 { が if es.closed() && n > 0 { に変更されました。
   • es.closed() メソッドは、isClosed の値をアトミックに読み取るために導入されました。これにより、Read メソッドが isClosed の状態をチェックする際に、他のゴルーチンによる同時書き込みの影響を受けなくなります。

4. Close メソッドの変更:

   • if es.isClosed { return nil } が if !es.setClosed() { return nil } に変更されました。
   • es.isClosed = true の直接的な代入が削除されました。
   • es.setClosed() メソッドは、isClosed の値をアトミックに true に設定するために導入されました。このメソッドは、isClosed がまだ false (0) の場合にのみ true (1) に設定し、その操作が成功したかどうかを返します。これにより、Close メソッドが複数回呼び出されても、実際のクローズ処理（es.body.Close() や es.condfn()）は一度だけ実行されることが保証され、競合状態が回避されます。

5. 新しいヘルパーメソッドの追加:

   • func (es *bodyEOFSignal) closed() bool: このメソッドは、atomic.LoadUint32(&es.isClosed) を使用して isClosed の値をアトミックに読み取ります。戻り値が 0 でない場合に true を返すことで、isClosed が true であることを安全に確認できます。
   • func (es *bodyEOFSignal) setClosed() bool: このメソッドは、atomic.CompareAndSwapUint32(&es.isClosed, 0, 1) を使用します。これは、isClosed の現在の値が 0（false）である場合にのみ、その値を 1（true）にアトミックに設定します。この操作が成功した場合（つまり、isClosed が false から true に変更された場合）は true を返し、既に true であった場合は false を返します。これにより、Close 処理の二重実行を防ぎつつ、スレッドセーフな状態遷移を実現します。

これらの変更により、bodyEOFSignal の isClosed フィールドへのアクセスはすべてアトミック操作によって行われるようになり、複数のゴルーチンからの同時アクセスによる競合状態が完全に解消されました。

関連リンク

• Go CL (Code Review): https://golang.org/cl/6640057
• GitHub Issue: Issue #4191: net/http: Unrecoverable client panic crashes my program

参考にした情報源リンク

• Go Issue 4191: https://github.com/golang/go/issues/4191 (Web検索結果より)
• Go Build Log: http://build.golang.org/log/61e43a328fb220801d3d5c88cd91916cfc5dc43c (Web検索結果より、古いビルドログであり直接アクセスはできないが、Issueとの関連性が示唆されている)
• Go sync/atomic package documentation: Go言語の公式ドキュメント（sync/atomic パッケージに関する一般的な情報）
• Go sync.Once documentation: Go言語の公式ドキュメント（sync.Once に関する一般的な情報）
• Go io package documentation: Go言語の公式ドキュメント（io.ReadCloser に関する一般的な情報）
• Go Concurrency Patterns: Go言語における並行処理の一般的な概念とパターンに関する情報