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

このコミットは、Go言語の標準ライブラリであるbufioパッケージにおける以前のバグ修正（CL 86220044 / 41388e58be65）を元に戻すものです。具体的には、bufio.ReaderのReadByteメソッドにおける潜在的な無限ループを修正しようとした試みが、誤った修正であったため、その変更を取り消しています。

コミット

• コミットハッシュ: 34a21dcae425a9353b7d763958c5039a0e767531
• 作者: Robert Griesemer gri@golang.org
• 日付: 2014年4月9日 水曜日 18:23:53 -0700
• コミットメッセージ:

  undo CL 86220044 / 41388e58be65
  
  bufio: undo incorrect bug fix
  
  ««« original CL description
  bufio: fix potential endless loop in ReadByte
  
  Fixes #7745.
  
  LGTM=bradfitz, r
  R=r, bradfitz
  CC=golang-codereviews
  https://golang.org/cl/86220044
  »»»
  
  LGTM=adg
  R=r, adg
  CC=golang-codereviews
  https://golang.org/cl/85550045
  

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

https://github.com/golang/go/commit/34a21dcae425a9353b7d763958c5039a0e767531

元コミット内容

このコミットが元に戻している元のコミット（CL 86220044 / 41388e58be65）の目的は、以下の通りでした。

bufio: fix potential endless loop in ReadByte Fixes #7745.

これは、bufioパッケージのReadByteメソッドに存在する可能性のある無限ループを修正しようとするものでした。

変更の背景

このコミットは、以前のコミット（CL 86220044 / 41388e58be65）によって導入されたバグ修正が「誤っていた」ために行われました。元の修正は、bufio.Reader.ReadByteが、基になるio.Readerが常に0バイトを返す場合に無限ループに陥る可能性があるという問題（Issue #7745）に対処しようとしたものです。

しかし、その修正は意図しない副作用をもたらしたか、あるいは問題の根本原因を正しく解決していなかったため、このコミットでその修正自体が取り消されることになりました。これは、複雑なI/O処理において、特定の条件下での挙動を正確に予測し、堅牢なコードを書くことの難しさを示しています。特に、io.Readerが0バイトを返すという挙動は、EOF（End Of File）ではないが、現時点ではデータがないという状態を示すため、これを適切に処理しないと、無限ループや不必要なリトライを引き起こす可能性があります。

前提知識の解説

このコミットを理解するためには、以下のGo言語の概念とbufioパッケージの動作に関する知識が必要です。

io.Reader インターフェース

Go言語における基本的なI/O操作のインターフェースです。

type Reader interface {
    Read(p []byte) (n int, err error)
}

Readメソッドは、データをpに読み込み、読み込んだバイト数nとエラーerrを返します。

• n > 0かつerr == nil: nバイトを正常に読み込んだ。
• n == 0かつerr == nil: 現時点では読み込むデータがないが、EOFではない。後でデータが利用可能になる可能性がある。これは、ノンブロッキングI/Oや、データがまだ到着していないネットワーク接続などで発生し得ます。
• n == 0かつerr == io.EOF: ストリームの終端に達した。これ以上データは読み込めない。
• n > 0かつerr != nil: nバイトを読み込んだが、その後にエラーが発生した。

特に重要なのは、n == 0かつerr == nilの場合です。これは「進捗がない」状態を示し、この状態が続くと無限ループに陥る可能性があります。

bufio パッケージ

bufioパッケージは、I/O操作をバッファリングすることで効率化するための機能を提供します。特に、bufio.Readerは、基になるio.Readerからの読み込みをバッファリングし、より小さな読み込み要求（例: ReadByte）を効率的に処理します。

bufio.Reader の fill() メソッド

bufio.Readerの内部メソッドであるfill()は、バッファが空になったときに、基になるio.Readerからデータを読み込んでバッファを埋める役割を担います。このメソッドが、io.Readerがn=0, err=nilを返す場合にどのように振る舞うかが、無限ループの問題に直結します。

io.ErrNoProgress

io.ErrNoProgressは、io.Readerが繰り返し0バイトを返し、かつエラーも返さない場合に、bufioパッケージのようなバッファリングリーダーが「進捗がない」ことを示すために使用されるエラーです。これは、無限ループを防ぐためのメカニズムとして導入されることがあります。

無限ループの概念

プログラムが特定の条件を満たし続ける限り、同じコードブロックを繰り返し実行し続ける状態を指します。I/O操作においては、データが利用可能になるのを待つループが、データが全く来ない、あるいは常に0バイトを返すような状況で適切に終了しない場合に発生します。

技術的詳細

このコミットは、src/pkg/bufio/bufio.goのfill()メソッドとRead()メソッド、そしてsrc/pkg/bufio/bufio_test.goのテストコードに変更を加えています。

bufio.go の変更点

元の修正では、fill()メソッド内にfor i := maxConsecutiveEmptyReads; i > 0; i--というループが導入されていました。このループは、基になるリーダーが連続して0バイトを返す場合に、一定回数（maxConsecutiveEmptyReads回）だけリトライし、それでもデータが読み込めない場合はio.ErrNoProgressを返すことで、無限ループを防ごうとしていました。

しかし、このコミットでは、そのforループ全体が削除され、fill()メソッドは基になるリーダーから一度だけ読み込みを試みる元のシンプルな形に戻されています。

--- a/src/pkg/bufio/bufio.go
+++ b/src/pkg/bufio/bufio.go
@@ -88,22 +88,15 @@ func (b *Reader) fill() {
 		b.r = 0
 	}

-	// Read new data: try a limited number of times.
-	for i := maxConsecutiveEmptyReads; i > 0; i-- {
-		n, err := b.rd.Read(b.buf[b.w:])
-		if n < 0 {
-			panic(errNegativeRead)
-		}
-		b.w += n
-		if err != nil {
-			b.err = err
-			return
-		}
-		if n > 0 {
-			return
-		}
+	// Read new data.
+	n, err := b.rd.Read(b.buf[b.w:])
+	if n < 0 {
+		panic(errNegativeRead)
 	}
-	b.err = io.ErrNoProgress
+	b.w += n
+	if err != nil {
+		b.err = err
+	}
 }

 func (b *Reader) readErr() error {
@@ -158,9 +151,6 @@ func (b *Reader) Read(p []byte) (n int, err error) {
 			// Large read, empty buffer.
 			// Read directly into p to avoid copy.
 			n, b.err = b.rd.Read(p)
-			if n < 0 {
-				panic(errNegativeRead)
-			}
 			if n > 0 {
 				b.lastByte = int(p[n-1])
 				b.lastRuneSize = -1

また、Read()メソッド内のif n < 0 { panic(errNegativeRead) }というチェックも削除されています。これは、io.Readerの規約上、Readメソッドが負のバイト数を返すことはないため、冗長なチェックと判断された可能性があります。

bufio_test.go の変更点

元の修正では、TestZeroReaderというテストケースが追加されていました。このテストは、常に0バイトを返すzeroReaderというカスタムリーダーを使用して、ReadByteが無限ループに陥らないことを検証しようとするものでした。具体的には、time.After(time.Second)でタイムアウトを設定し、無限ループが発生しないことを確認していました。

このコミットでは、そのTestZeroReaderテストケース全体が削除されています。これは、元の修正が誤っていたため、その修正を検証するためのテストも不要になったことを意味します。

--- a/src/pkg/bufio/bufio_test.go
+++ b/src/pkg/bufio/bufio_test.go
@@ -14,7 +14,6 @@ import (
 	"strings"
 	"testing"
 	"testing/iotest"
-	"time"
 	"unicode/utf8"
 )

@@ -175,34 +174,6 @@ func TestReader(t *testing.T) {
 	}
 }

-type zeroReader struct{}
-
-func (zeroReader) Read(p []byte) (int, error) {
-	return 0, nil
-}
-
-func TestZeroReader(t *testing.T) {
-	var z zeroReader
-	r := NewReader(z)
-
-	c := make(chan error)
-	go func() {
-		_, err := r.ReadByte()
-		c <- err
-	}()
-
-	select {
-	case err := <-c:
-		if err == nil {
-			t.Error("error expected")
-		} else if err != io.ErrNoProgress {
-			t.Error("unexpected error:", err)
-		}
-	case <-time.After(time.Second):
-		t.Error("test timed out (endless loop in ReadByte?)")
-	}
-}
-
 // A StringReader delivers its data one string segment at a time via Read.
 type StringReader struct {
 	data []string

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

src/pkg/bufio/bufio.go

// func (b *Reader) fill()
// 変更前:
// 	// Read new data: try a limited number of times.
// 	for i := maxConsecutiveEmptyReads; i > 0; i-- {
// 		n, err := b.rd.Read(b.buf[b.w:])
// 		if n < 0 {
// 			panic(errNegativeRead)
// 		}
// 		b.w += n
// 		if err != nil {
// 			b.err = err
// 			return
// 		}
// 		if n > 0 {
// 			return
// 		}
// 	}
// 	b.err = io.ErrNoProgress
//
// 変更後:
// 	// Read new data.
// 	n, err := b.rd.Read(b.buf[b.w:])
// 	if n < 0 {
// 		panic(errNegativeRead)
// 	}
// 	b.w += n
// 	if err != nil {
// 		b.err = err
// 	}

// func (b *Reader) Read(p []byte) (n int, err error)
// 変更前:
// 			if n < 0 {
// 				panic(errNegativeRead)
// 			}
// 変更後: (削除)

src/pkg/bufio/bufio_test.go

// TestZeroReader 関数全体が削除されました。
// 変更前:
// type zeroReader struct{}
//
// func (zeroReader) Read(p []byte) (int, error) {
// 	return 0, nil
// }
//
// func TestZeroReader(t *testing.T) {
// 	var z zeroReader
// 	r := NewReader(z)
//
// 	c := make(chan error)
// 	go func() {
// 		_, err := r.ReadByte()
// 		c <- err
// 	}()
//
// 	select {
// 	case err := <-c:
// 		if err == nil {
// 			t.Error("error expected")
// 		} else if err != io.ErrNoProgress {
// 			t.Error("unexpected error:", err)
// 		}
// 	case <-time.After(time.Second):
// 		t.Error("test timed out (endless loop in ReadByte?)")
// 	}
// }
//
// 変更後: (削除)

コアとなるコードの解説

bufio.go の fill() メソッドの変更

この変更の核心は、fill()メソッドからmaxConsecutiveEmptyReadsを使ったリトライループが削除されたことです。

• 削除されたループの意図: 元の修正では、基になるio.Readerがn=0, err=nilを連続して返す場合に、bufio.Readerが無限にReadを呼び出し続けるのを防ぐために、このループが導入されました。一定回数リトライしても進捗がない場合、io.ErrNoProgressを返すことで、呼び出し元に「これ以上読み込めない」ことを伝えていました。
• ループ削除の理由: このループが削除されたということは、このアプローチが正しくなかった、あるいは新たな問題を引き起こしたと判断されたことを意味します。io.Readerの規約では、n=0, err=nilは「現時点ではデータがないが、後で利用可能になる可能性がある」ことを示します。この場合、bufio.Readerはブロックするか、あるいは呼び出し元に制御を戻して、呼び出し元が再試行するかどうかを決定できるようにすべきです。連続リトライのロジックは、特定のシナリオではデッドロックや不必要なCPU使用率の増加を引き起こす可能性があったのかもしれません。
• panic(errNegativeRead) の削除: Read()メソッドからif n < 0 { panic(errNegativeRead) }が削除されたのは、io.Readerの規約上、Readメソッドが負の値を返すことはないため、このチェックが不要であると判断されたためです。これはコードの簡潔化と、Goのインターフェース規約への信頼を示しています。

bufio_test.go の TestZeroReader の削除

TestZeroReaderは、元の修正が正しく機能するかどうかを検証するために書かれたテストでした。このテストは、Readメソッドが常に0バイトを返すような特殊なio.Reader（zeroReader）を作成し、bufio.Reader.ReadByte()が無限ループに陥らず、最終的にio.ErrNoProgressを返すことを期待していました。

このテストケースが削除されたのは、元の修正自体が取り消されたため、その修正を検証するテストも不要になったからです。これは、テストコードが本番コードの変更に密接に結合している典型的な例であり、本番コードの変更が取り消されると、関連するテストも削除されるべきであることを示しています。

関連リンク

• 元の変更リスト (CL): https://golang.org/cl/86220044 (このコミットが元に戻した変更)
• このコミットの変更リスト (CL): https://golang.org/cl/85550045
• 関連するIssue: Fixes #7745 (元の修正が対処しようとした問題)

参考にした情報源リンク

• Go言語のio.Readerインターフェースに関する公式ドキュメントや解説記事
• Go言語のbufioパッケージに関する公式ドキュメントや解説記事
• io.ErrNoProgressに関するGo言語のドキュメントや議論
• Go言語のコミット履歴と関連するIssueトラッカーの議論 (特に #7745)
• GitHubのコミットページ: https://github.com/golang/go/commit/34a21dcae425a9353b7d763958c5039a0e767531
• GitHubのコミットページ (元に戻されたコミット): https://github.com/golang/go/commit/41388e58be65 (ただし、このURLはアクセスできなかったため、コミットメッセージから情報を推測しました)