[インデックス 19109] ファイルの概要
このコミットは、Go言語の標準ライブラリであるbufioパッケージにおける重要なバグ修正と改善を含んでいます。具体的には、bufio.ReaderのReadByteメソッドが特定の条件下で無限ループに陥る可能性があった問題に対処し、ReadSliceの実装を簡素化しています。また、fill()メソッドがバッファが満杯の状態で呼び出されないように修正することで、net/textprotoパッケージのテスト失敗(TestLargeReadMIMEHeader)も解決しています。
コミット
commit 7b6bc3ebb3a4088506d3d9c324d85aa49c035074
Author: Robert Griesemer <gri@golang.org>
Date: Thu Apr 10 21:46:00 2014 -0700
bufio: fix potential endless loop in ReadByte
Also: Simplify ReadSlice implementation and
ensure that it doesn't call fill() with a full
buffer (this caused a failure in net/textproto
TestLargeReadMIMEHeader because fill() wasn't able
to read more data).
Fixes #7745.
LGTM=bradfitz
R=r, bradfitz
CC=golang-codereviews
https://golang.org/cl/86590043
GitHub上でのコミットページへのリンク
https://github.com/golang/go/commit/7b6bc3ebb3a4088506d3d9c324d85aa49c035074
元コミット内容
このコミットの元のメッセージは以下の通りです。
「bufio: ReadByteにおける潜在的な無限ループを修正。 また、ReadSliceの実装を簡素化し、 バッファが満杯の状態でfill()を呼び出さないようにする(これにより、fill()がこれ以上データを読み取れなかったため、net/textprotoのTestLargeReadMIMEHeaderで失敗が発生していた)。 Fixes #7745。」
変更の背景
このコミットは、主に以下の2つの問題に対処するために行われました。
-
bufio.Reader.ReadByteの無限ループ問題 (Issue #7745):bufio.Readerは、内部バッファを使用してI/O操作を効率化します。ReadByteメソッドが呼び出され、内部バッファに読み取るべきバイトがない場合、bufio.Readerはラップしている基盤のio.Readerからデータを読み込むためにfill()メソッドを呼び出します。 問題は、基盤のio.ReaderがReadメソッドの呼び出しに対して、常に「0バイト読み込み、エラーなし ((0, nil))」を返した場合に発生しました。io.Readerの契約では、Readメソッドは通常、データを読み込むか、エラー(io.EOFなど)を返すことが期待されます。しかし、一部の不正な、または特殊なio.Reader実装が0, nilを返し続けると、bufio.Readerはデータが読み込まれたと誤解し、無限にfill()を呼び出し続け、結果としてReadByteが無限ループに陥る可能性がありました。これは、特にネットワーク通信などでタイムアウトが発生する原因となり、アプリケーションのハングやリソース枯渇を引き起こす可能性がありました。 -
net/textproto.TestLargeReadMIMEHeaderのテスト失敗:bufio.Readerのfill()メソッドが、バッファがすでに満杯であるにもかかわらず呼び出される可能性がありました。これは、特にReadSliceのようなメソッドが、バッファの空き容量を適切に確認せずにfill()を呼び出すことで発生し得ました。fill()は、バッファに空きがあることを前提としてデータを読み込もうとするため、バッファが満杯の状態で呼び出されると、期待通りに動作せず、結果としてnet/textprotoパッケージのTestLargeReadMIMEHeaderのようなテストが失敗していました。これは、fill()がこれ以上データを読み取れない状況で、さらに読み取りを試みるという非効率性もはらんでいました。
これらの問題を解決し、bufioパッケージの堅牢性と信頼性を向上させることが、このコミットの主な目的です。
前提知識の解説
このコミットの変更内容を理解するためには、以下のGo言語の概念とbufioパッケージの動作に関する知識が必要です。
-
io.Readerインターフェース: Go言語における基本的な入力インターフェースです。Read(p []byte) (n int, err error)メソッドを持ち、データをバイトスライスpに読み込み、読み込んだバイト数nとエラーerrを返します。Readメソッドは、以下のいずれかを返すことが期待されます。n > 0, nil:nバイトのデータを読み込み成功。0, io.EOF: ストリームの終端に達した。0, err: エラーが発生し、データは読み込まれなかった。n > 0, err:nバイトのデータを読み込み、その後エラーが発生した(通常はio.EOF)。 この契約に反して、0, nilを繰り返し返すio.Readerは「進捗のないリーダー」と見なされ、問題を引き起こす可能性があります。
-
bufioパッケージ:bufioパッケージは、io.Readerやio.Writerインターフェースをラップして、バッファリングされたI/O操作を提供します。これにより、基盤となるI/O操作の回数を減らし、パフォーマンスを向上させることができます。bufio.Reader: バッファリングされた読み取りを提供します。内部にバイトスライス(バッファ)を持ち、基盤のio.Readerから一度に大量のデータを読み込み、それを小分けにしてユーザーに提供します。b.buf:bufio.Readerの内部バッファ。b.r(read index): バッファ内で次に読み取るべきバイトのインデックス。b.w(write index): バッファ内で次に書き込むべきバイトのインデックス(基盤のリーダーから読み込んだデータの終端)。b.Buffered(): 現在バッファに読み取り可能なバイト数を返します (b.w - b.r)。b.fill(): 内部バッファが空になったとき、またはより多くのデータが必要なときに、基盤のio.Readerからデータを読み込んでバッファを埋めるための内部メソッド。
-
io.ErrNoProgress: Go 1.3で導入されたエラーで、io.ReaderがReadメソッドの呼び出しに対して繰り返し0, nilを返し、進捗がないことを示すために使用されます。これは、無限ループを防ぐための安全策として機能します。 -
ReadByte():bufio.Readerのメソッドで、バッファから1バイトを読み取って返します。バッファが空の場合、fill()を呼び出してバッファを埋めようとします。 -
ReadSlice(delim byte):bufio.Readerのメソッドで、指定された区切り文字delimが見つかるまでデータを読み込み、その区切り文字を含むバイトスライスを返します。内部バッファに区切り文字が見つからない場合、fill()を呼び出してさらにデータを読み込もうとします。
技術的詳細
このコミットは、bufio.Readerの以下の主要なメソッドに焦点を当てて修正を行っています。
-
fill()メソッドの変更:- 無限ループの防止: 以前の
fill()は、基盤のio.Readerが0, nilを返した場合、無限にReadを呼び出し続ける可能性がありました。この修正では、maxConsecutiveEmptyReads(連続して空の読み取りが許容される最大回数)という定数(おそらく内部的に定義されている)を導入し、ループカウンタとして使用しています。fill()は、基盤のio.Readerがn > 0(データが読み込まれた)を返すか、エラーが発生するまで、またはmaxConsecutiveEmptyReads回連続で0, nilを返した場合にループを終了します。 io.ErrNoProgressの導入:maxConsecutiveEmptyReads回連続で0, nilが返された場合、b.errにio.ErrNoProgressを設定します。これにより、bufio.Readerの他のメソッドがこのエラーを検出し、無限ループを回避できるようになります。- バッファ満杯チェックの追加:
fill()の冒頭にif b.w >= len(b.buf) { panic("bufio: tried to fill full buffer") }というチェックが追加されました。これは、fill()がバッファが満杯の状態で呼び出されるべきではないという前提を強制するためのパニックです。これにより、ReadSliceのような呼び出し元がfill()を不適切に呼び出すことを防ぎます。
- 無限ループの防止: 以前の
-
ReadByte()メソッドの変更:ReadByte()は、バッファが空の場合にb.fill()を呼び出す前に、// buffer is emptyというコメントを追加し、fill()が適切な状況で呼び出されることを明確にしています。fill()の変更により、ReadByteは基盤のリーダーが進捗しない場合にio.ErrNoProgressを受け取るようになり、無限ループが解消されます。
-
ReadSlice()メソッドの簡素化と修正:- 以前の
ReadSliceは、バッファ内に区切り文字が見つからない場合に、バッファを埋めてから再度検索するという複雑なロジックを持っていました。 - 新しい実装では、無限ループ内で以下の処理を繰り返します。
- まず、現在のバッファ範囲(
b.buf[b.r:b.w])内で区切り文字を検索します。見つかれば、その部分をlineとして返し、b.rを更新して終了します。 - 保留中のエラー(
b.err != nil)がある場合、残りのバッファ内容を返し、エラーを伝播します。 - バッファが満杯かどうかのチェック:
if n := b.Buffered(); n >= len(b.buf)という条件でバッファが満杯かどうかを確認します。満杯であれば、ErrBufferFullを返します。これは、ReadSliceがバッファの容量を超えて読み取ろうとしないようにするための重要な変更です。 b.fill()の呼び出し: 上記のチェックを通過した場合(つまり、バッファが満杯ではない場合)にのみb.fill()を呼び出します。これにより、fill()がバッファ満杯の状態で呼び出されることがなくなり、net/textprotoのテスト失敗が解決されます。コメント// buffer is not fullが追加され、この前提が強調されています。
- まず、現在のバッファ範囲(
- 以前の
-
ReadRune()メソッドの変更:ReadRune()のfill()呼び出し条件に&& b.w-b.r < len(b.buf)が追加されました。これは、ReadSliceと同様に、fill()がバッファが満杯の状態で呼び出されないようにするためのガードです。
-
WriteTo()メソッドの変更:WriteTo()のループ条件が変更され、b.fill()の呼び出しタイミングが調整されました。以前はループ内で常にb.fill()を呼び出していましたが、新しいコードでは、バッファが空ではない場合にのみwriteBufを呼び出し、その後b.fill()を呼び出すように変更されています。これにより、fill()がより意図されたタイミングで呼び出されるようになります。
-
writeBuf()メソッドの変更:w.Writeが実際に書き込んだバイト数nが、書き込もうとしたバイト数(b.r-b.w、ただしこれはb.w-b.rの誤記の可能性が高い、またはb.buf[b.r:b.w]の長さ)よりも少ない場合にパニックを発生させるチェックが追加されました。これは、io.Writerの契約違反を早期に検出するための堅牢性向上です。
-
テストケースの追加 (
bufio_test.go):TestZeroReaderという新しいテストケースが追加されました。このテストは、Readメソッドが常に0, nilを返すzeroReaderというカスタムio.Readerを定義し、bufio.Reader.ReadByte()が無限ループに陥らないことを検証します。具体的には、ReadByte()の呼び出しをゴルーチンで実行し、タイムアウトを設定することで、無限ループが発生しないことを確認しています。これにより、Issue #7745で報告された問題が実際に修正されたことを保証します。
これらの変更は、bufio.Readerの内部状態管理と基盤のio.Readerとの相互作用をより厳密にし、予期せぬ動作(特に無限ループ)を防ぐためのものです。
コアとなるコードの変更箇所
src/pkg/bufio/bufio.go
--- a/src/pkg/bufio/bufio.go
+++ b/src/pkg/bufio/bufio.go
@@ -88,15 +88,26 @@ func (b *Reader) fill() {
b.r = 0
}
- // Read new data.
- n, err := b.rd.Read(b.buf[b.w:])
- if n < 0 {
- panic(errNegativeRead)
+ if b.w >= len(b.buf) {
+ panic("bufio: tried to fill full buffer")
}
- b.w += n
- if err != nil {
- b.err = err
+
+ // 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
}
func (b *Reader) readErr() error {
@@ -116,8 +127,9 @@ func (b *Reader) Peek(n int) ([]byte, error) {
if n > len(b.buf) {
return nil, ErrBufferFull
}
+ // 0 <= n <= len(b.buf)
for b.w-b.r < n && b.err == nil {
- b.fill()
+ b.fill() // b.w-b.r < len(b.buf) => buffer is not full
}
m := b.w - b.r
if m > n {
@@ -143,7 +155,7 @@ func (b *Reader) Read(p []byte) (n int, err error) {
if n == 0 {
return 0, b.readErr()
}
- if b.w == b.r {
+ if b.r == b.w {
if b.err != nil {
return 0, b.readErr()
}
@@ -151,13 +163,16 @@ func (b *Reader) Read(p []byte) (n int, err error) {
if len(p) >= len(b.buf) {
// 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
}
return n, b.readErr()
}
- b.fill()
+ b.fill() // buffer is empty
if b.w == b.r {
return 0, b.readErr()
}
@@ -181,7 +196,7 @@ func (b *Reader) ReadByte() (c byte, err error) {
if b.err != nil {
return 0, b.readErr()
}
- b.fill()
+ b.fill() // buffer is empty
}
c = b.buf[b.r]
b.r++
@@ -211,8 +226,8 @@ func (b *Reader) UnreadByte() error {
// rune and its size in bytes. If the encoded rune is invalid, it consumes one byte
// and returns unicode.ReplacementChar (U+FFFD) with a size of 1.
func (b *Reader) ReadRune() (r rune, size int, err error) {
- for b.r+utf8.UTFMax > b.w && !utf8.FullRune(b.buf[b.r:b.w]) && b.err == nil {
- b.fill()
+ for b.r+utf8.UTFMax > b.w && !utf8.FullRune(b.buf[b.r:b.w]) && b.err == nil && b.w-b.r < len(b.buf) {
+ b.fill() // b.w-b.r < len(buf) => buffer is not full
}
b.lastRuneSize = -1
if b.r == b.w {
@@ -256,36 +271,28 @@ func (b *Reader) Buffered() int { return b.w - b.r }
// ReadBytes or ReadString instead.
// ReadSlice returns err != nil if and only if line does not end in delim.
func (b *Reader) ReadSlice(delim byte) (line []byte, err error) {
- // Look in buffer.
- if i := bytes.IndexByte(b.buf[b.r:b.w], delim); i >= 0 {
- line1 := b.buf[b.r : b.r+i+1]
- b.r += i + 1
- return line1, nil
- }
-
- // Read more into buffer, until buffer fills or we find delim.
for {
+ // Search buffer.
+ if i := bytes.IndexByte(b.buf[b.r:b.w], delim); i >= 0 {
+ line := b.buf[b.r : b.r+i+1]
+ b.r += i + 1
+ return line, nil
+ }
+
+ // Pending error?
if b.err != nil {
line := b.buf[b.r:b.w]
b.r = b.w
return line, b.readErr()
}
- n := b.Buffered()
- b.fill()
-
- // Search new part of buffer
- if i := bytes.IndexByte(b.buf[n:b.w], delim); i >= 0 {
- line := b.buf[0 : n+i+1]
- b.r = n + i + 1
- return line, nil
- }
-
- // Buffer is full?
- if b.Buffered() >= len(b.buf) {
+ // Buffer full?
+ if n := b.Buffered(); n >= len(b.buf) {
b.r = b.w
return b.buf, ErrBufferFull
}
+
+ b.fill() // buffer is not full
}
}
@@ -417,12 +424,18 @@ func (b *Reader) WriteTo(w io.Writer) (n int64, err error) {
return n, err
}
- for b.fill(); b.r < b.w; b.fill() {
+ if b.w-b.r < len(b.buf) {
+ b.fill() // buffer not full
+ }
+
+ for b.r < b.w {
+ // b.r < b.w => buffer is not empty
m, err := b.writeBuf(w)
n += m
if err != nil {
return n, err
}
+ b.fill() // buffer is empty
}
if b.err == io.EOF {
@@ -435,6 +448,9 @@ func (b *Reader) WriteTo(w io.Writer) (n int64, err error) {
// writeBuf writes the Reader's buffer to the writer.
func (b *Reader) writeBuf(w io.Writer) (int64, error) {
n, err := w.Write(b.buf[b.r:b.w])
+ if n < b.r-b.w {
+ panic(errors.New("bufio: writer did not write all data"))
+ }
b.r += n
return int64(n), err
}
src/pkg/bufio/bufio_test.go
--- a/src/pkg/bufio/bufio_test.go
+++ b/src/pkg/bufio/bufio_test.go
@@ -14,6 +14,7 @@ import (
"strings"
"testing"
"testing/iotest"
+ "time"
"unicode/utf8"
)
@@ -174,6 +175,34 @@ 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
-
fill()メソッド:if b.w >= len(b.buf) { panic("bufio: tried to fill full buffer") }: この行は、fill()が呼び出される前にバッファが満杯でないことを保証するためのガードです。もしバッファが満杯の状態でfill()が呼び出された場合、これはプログラミングエラーと見なされ、パニックが発生します。これにより、ReadSliceのような呼び出し元がfill()を不適切に呼び出すことを防ぎ、net/textprotoのテスト失敗の原因となっていた問題を解決します。for i := maxConsecutiveEmptyReads; i > 0; i--: ここが無限ループを防ぐための主要な変更点です。maxConsecutiveEmptyReadsは、基盤のio.Readerが0, nilを連続して返すことを許容する最大回数を定義します。ループ内でb.rd.Read(b.buf[b.w:])を呼び出し、以下の条件でループを終了します。n < 0: 負のバイト数が返された場合(不正なリーダーの動作)、パニック。err != nil: エラーが発生した場合、b.errにエラーを設定してreturn。n > 0: データが読み込まれた場合、return。
b.err = io.ErrNoProgress: ループがmaxConsecutiveEmptyReads回繰り返され、その間ずっと0, nilが返され続けた場合、つまり基盤のリーダーが進捗しないと判断された場合、b.errにio.ErrNoProgressを設定します。これにより、bufio.Readerの他のメソッドがこの状態を検出し、無限ループを回避できます。
-
ReadByte()メソッド:b.fill() // buffer is empty:ReadByteがバッファから読み取れない場合にfill()を呼び出す箇所です。fill()の変更により、ここで無限ループに陥る可能性がなくなりました。
-
ReadRune()メソッド:for b.r+utf8.UTFMax > b.w && !utf8.FullRune(b.buf[b.r:b.w]) && b.err == nil && b.w-b.r < len(b.buf):fill()を呼び出す条件に&& b.w-b.r < len(b.buf)が追加されました。これは、ReadRuneがfill()を呼び出す際に、バッファが満杯でないことを保証するためのものです。これにより、fill()がバッファ満杯の状態で呼び出されることを防ぎます。
-
ReadSlice()メソッド:- このメソッドは大幅に簡素化されました。以前はバッファの検索と
fill()の呼び出しが複雑に絡み合っていましたが、新しい実装では無限ループ内で以下のロジックを明確に実行します。if i := bytes.IndexByte(b.buf[b.r:b.w], delim); i >= 0: まず、現在のバッファ内で区切り文字を検索します。見つかれば、その部分を返し、b.rを更新します。if b.err != nil: 保留中のエラーがあれば、それを処理して返します。if n := b.Buffered(); n >= len(b.buf): 重要な変更点。バッファが満杯であるかどうかをチェックします。もし満杯であれば、ErrBufferFullを返します。これにより、ReadSliceがバッファの容量を超えて読み取ろうとすることを防ぎます。b.fill() // buffer is not full: 上記のチェックを通過した場合(つまり、バッファが満杯ではない場合)にのみfill()を呼び出します。これにより、fill()がバッファ満杯の状態で呼び出されることがなくなり、net/textprotoのテスト失敗が解決されます。
- このメソッドは大幅に簡素化されました。以前はバッファの検索と
-
WriteTo()メソッド:if b.w-b.r < len(b.buf) { b.fill() // buffer not full }:WriteToの開始時に、バッファに空きがある場合に一度fill()を呼び出すように変更されました。for b.r < b.w { ... b.fill() // buffer is empty }: ループ内で、バッファが空ではない場合にwriteBufを呼び出し、その後b.fill()を呼び出すように変更されました。これにより、fill()の呼び出しがより制御され、バッファが空になった後にのみ新しいデータを読み込むようになります。
-
writeBuf()メソッド:if n < b.r-b.w { panic(errors.New("bufio: writer did not write all data")) }:w.Writeが書き込もうとしたデータ量よりも少ないバイト数を返した場合にパニックを発生させるチェックが追加されました。これは、io.Writerの契約(要求されたバイト数を書き込むか、エラーを返す)に違反する実装を早期に検出するための堅牢性向上です。
src/pkg/bufio/bufio_test.go
TestZeroReader関数:type zeroReader struct{}:Readメソッドが常に0, nilを返すカスタムio.Readerを定義しています。func (zeroReader) Read(p []byte) (int, error) { return 0, nil }: このReadメソッドの実装が、Issue #7745で問題となった「進捗のないリーダー」の挙動を模倣しています。go func() { _, err := r.ReadByte(); c <- err }():bufio.ReaderのReadByte()メソッドを別のゴルーチンで実行します。select { ... case <-time.After(time.Second): t.Error("test timed out (endless loop in ReadByte?)") }:ReadByte()の呼び出しが1秒以内に完了し、io.ErrNoProgressエラーを返すことを期待しています。もし1秒を超えても完了しない場合、それは無限ループに陥っていることを意味し、テストは失敗します。これにより、ReadByteの無限ループ問題が修正されたことを自動的に検証できます。
これらの変更は、bufioパッケージの堅牢性を大幅に向上させ、特に不正なio.Readerの実装や特定のバッファリングシナリオにおける予期せぬ動作を防ぐことに貢献しています。
関連リンク
- Go Issue #7745: https://github.com/golang/go/issues/7745
- Go CL 86590043: https://golang.org/cl/86590043 (このコミットに対応するGoのコードレビューリンク)
参考にした情報源リンク
- Go言語
bufioパッケージのドキュメント: https://pkg.go.dev/bufio - Go言語
ioパッケージのドキュメント: https://pkg.go.dev/io io.ErrNoProgressに関するGoのドキュメント: https://pkg.go.dev/io#pkg-variables- Stack Overflow: "When does bufio.Reader return io.ErrNoProgress?": https://stackoverflow.com/questions/24073342/when-does-bufio-reader-return-io-errnoprogress
- Go言語の
io.Readerインターフェースの契約に関する議論: https://github.com/golang/go/issues/1993I have generated the detailed explanation in Markdown format, following all the specified instructions and chapter structure. I have included background, prerequisite knowledge, and technical details, and used the information gathered from the commit data and web searches. The output is in Japanese and is highly detailed. The explanation is now ready to be outputted.
# [インデックス 19109] ファイルの概要
このコミットは、Go言語の標準ライブラリである`bufio`パッケージにおける重要なバグ修正と改善を含んでいます。具体的には、`bufio.Reader`の`ReadByte`メソッドが特定の条件下で無限ループに陥る可能性があった問題に対処し、`ReadSlice`の実装を簡素化しています。また、`fill()`メソッドがバッファが満杯の状態で呼び出されないように修正することで、`net/textproto`パッケージのテスト失敗(`TestLargeReadMIMEHeader`)も解決しています。
## コミット
commit 7b6bc3ebb3a4088506d3d9c324d85aa49c035074 Author: Robert Griesemer gri@golang.org Date: Thu Apr 10 21:46:00 2014 -0700
bufio: fix potential endless loop in ReadByte
Also: Simplify ReadSlice implementation and
ensure that it doesn't call fill() with a full
buffer (this caused a failure in net/textproto
TestLargeReadMIMEHeader because fill() wasn't able
to read more data).
Fixes #7745.
LGTM=bradfitz
R=r, bradfitz
CC=golang-codereviews
https://golang.org/cl/86590043
## GitHub上でのコミットページへのリンク
[https://github.com/golang/go/commit/7b6bc3ebb3a4088506d3d9c324d85aa49c035074](https://github.com/golang/go/commit/7b6bc3ebb3a4088506d3d9c324d85aa49c035074)
## 元コミット内容
このコミットの元のメッセージは以下の通りです。
「bufio: ReadByteにおける潜在的な無限ループを修正。
また、ReadSliceの実装を簡素化し、
バッファが満杯の状態でfill()を呼び出さないようにする(これにより、fill()がこれ以上データを読み取れなかったため、net/textprotoのTestLargeReadMIMEHeaderで失敗が発生していた)。
Fixes #7745。」
## 変更の背景
このコミットは、主に以下の2つの問題に対処するために行われました。
1. **`bufio.Reader.ReadByte`の無限ループ問題 (Issue #7745)**:
`bufio.Reader`は、内部バッファを使用してI/O操作を効率化します。`ReadByte`メソッドが呼び出され、内部バッファに読み取るべきバイトがない場合、`bufio.Reader`はラップしている基盤の`io.Reader`からデータを読み込むために`fill()`メソッドを呼び出します。
問題は、基盤の`io.Reader`が`Read`メソッドの呼び出しに対して、常に「0バイト読み込み、エラーなし (`(0, nil)`)」を返した場合に発生しました。`io.Reader`の契約では、`Read`メソッドは通常、データを読み込むか、エラー(`io.EOF`など)を返すことが期待されます。しかし、一部の不正な、または特殊な`io.Reader`実装が`0, nil`を返し続けると、`bufio.Reader`はデータが読み込まれたと誤解し、無限に`fill()`を呼び出し続け、結果として`ReadByte`が無限ループに陥る可能性がありました。これは、特にネットワーク通信などでタイムアウトが発生する原因となり、アプリケーションのハングやリソース枯渇を引き起こす可能性がありました。
2. **`net/textproto.TestLargeReadMIMEHeader`のテスト失敗**:
`bufio.Reader`の`fill()`メソッドが、バッファがすでに満杯であるにもかかわらず呼び出される可能性がありました。これは、特に`ReadSlice`のようなメソッドが、バッファの空き容量を適切に確認せずに`fill()`を呼び出すことで発生し得ました。`fill()`は、バッファに空きがあることを前提としてデータを読み込もうとするため、バッファが満杯の状態で呼び出されると、期待通りに動作せず、結果として`net/textproto`パッケージの`TestLargeReadMIMEHeader`のようなテストが失敗していました。これは、`fill()`がこれ以上データを読み取れない状況で、さらに読み取りを試みるという非効率性もはらんでいました。
これらの問題を解決し、`bufio`パッケージの堅牢性と信頼性を向上させることが、このコミットの主な目的です。
## 前提知識の解説
このコミットの変更内容を理解するためには、以下のGo言語の概念と`bufio`パッケージの動作に関する知識が必要です。
* **`io.Reader`インターフェース**:
Go言語における基本的な入力インターフェースです。`Read(p []byte) (n int, err error)`メソッドを持ち、データをバイトスライス`p`に読み込み、読み込んだバイト数`n`とエラー`err`を返します。`Read`メソッドは、以下のいずれかを返すことが期待されます。
* `n > 0, nil`: `n`バイトのデータを読み込み成功。
* `0, io.EOF`: ストリームの終端に達した。
* `0, err`: エラーが発生し、データは読み込まれなかった。
* `n > 0, err`: `n`バイトのデータを読み込み、その後エラーが発生した(通常は`io.EOF`)。
この契約に反して、`0, nil`を繰り返し返す`io.Reader`は「進捗のないリーダー」と見なされ、問題を引き起こす可能性があります。
* **`bufio`パッケージ**:
`bufio`パッケージは、`io.Reader`や`io.Writer`インターフェースをラップして、バッファリングされたI/O操作を提供します。これにより、基盤となるI/O操作の回数を減らし、パフォーマンスを向上させることができます。
* **`bufio.Reader`**: バッファリングされた読み取りを提供します。内部にバイトスライス(バッファ)を持ち、基盤の`io.Reader`から一度に大量のデータを読み込み、それを小分けにしてユーザーに提供します。
* **`b.buf`**: `bufio.Reader`の内部バッファ。
* **`b.r` (read index)**: バッファ内で次に読み取るべきバイトのインデックス。
* **`b.w` (write index)**: バッファ内で次に書き込むべきバイトのインデックス(基盤のリーダーから読み込んだデータの終端)。
* **`b.Buffered()`**: 現在バッファに読み取り可能なバイト数を返します (`b.w - b.r`)。
* **`b.fill()`**: 内部バッファが空になったとき、またはより多くのデータが必要なときに、基盤の`io.Reader`からデータを読み込んでバッファを埋めるための内部メソッド。
* **`io.ErrNoProgress`**:
Go 1.3で導入されたエラーで、`io.Reader`が`Read`メソッドの呼び出しに対して繰り返し`0, nil`を返し、進捗がないことを示すために使用されます。これは、無限ループを防ぐための安全策として機能します。
* **`ReadByte()`**:
`bufio.Reader`のメソッドで、バッファから1バイトを読み取って返します。バッファが空の場合、`fill()`を呼び出してバッファを埋めようとします。
* **`ReadSlice(delim byte)`**:
`bufio.Reader`のメソッドで、指定された区切り文字`delim`が見つかるまでデータを読み込み、その区切り文字を含むバイトスライスを返します。内部バッファに区切り文字が見つからない場合、`fill()`を呼び出してさらにデータを読み込もうとします。
## 技術的詳細
このコミットは、`bufio.Reader`の以下の主要なメソッドに焦点を当てて修正を行っています。
1. **`fill()` メソッドの変更**:
* **無限ループの防止**: 以前の`fill()`は、基盤の`io.Reader`が`0, nil`を返した場合、無限に`Read`を呼び出し続ける可能性がありました。この修正では、`maxConsecutiveEmptyReads`(連続して空の読み取りが許容される最大回数)という定数(おそらく内部的に定義されている)を導入し、ループカウンタとして使用しています。`fill()`は、基盤の`io.Reader`が`n > 0`(データが読み込まれた)を返すか、エラーが発生するまで、または`maxConsecutiveEmptyReads`回連続で`0, nil`を返した場合にループを終了します。
* **`io.ErrNoProgress`の導入**: `maxConsecutiveEmptyReads`回連続で`0, nil`が返された場合、`b.err`に`io.ErrNoProgress`を設定します。これにより、`bufio.Reader`の他のメソッドがこのエラーを検出し、無限ループを回避できるようになります。
* **バッファ満杯チェックの追加**: `fill()`の冒頭に`if b.w >= len(b.buf) { panic("bufio: tried to fill full buffer") }`というチェックが追加されました。これは、`fill()`がバッファが満杯の状態で呼び出されるべきではないという前提を強制するためのパニックです。これにより、`ReadSlice`のような呼び出し元が`fill()`を不適切に呼び出すことを防ぎます。
2. **`ReadByte()` メソッドの変更**:
* `ReadByte()`は、バッファが空の場合に`b.fill()`を呼び出す前に、`// buffer is empty`というコメントを追加し、`fill()`が適切な状況で呼び出されることを明確にしています。`fill()`の変更により、`ReadByte`は基盤のリーダーが進捗しない場合に`io.ErrNoProgress`を受け取るようになり、無限ループが解消されます。
3. **`ReadSlice()` メソッドの簡素化と修正**:
* 以前の`ReadSlice`は、バッファ内に区切り文字が見つからない場合に、バッファを埋めてから再度検索するという複雑なロジックを持っていました。
* 新しい実装では、無限ループ内で以下の処理を繰り返します。
* まず、現在のバッファ範囲(`b.buf[b.r:b.w]`)内で区切り文字を検索します。見つかれば、その部分を`line`として返し、`b.r`を更新して終了します。
* 保留中のエラー(`b.err != nil`)がある場合、残りのバッファ内容を返し、エラーを伝播します。
* **バッファが満杯かどうかのチェック**: `if n := b.Buffered(); n >= len(b.buf)`という条件でバッファが満杯かどうかを確認します。満杯であれば、`ErrBufferFull`を返します。これは、`ReadSlice`がバッファの容量を超えて読み取ろうとしないようにするための重要な変更です。
* **`b.fill()`の呼び出し**: 上記のチェックを通過した場合(つまり、バッファが満杯ではない場合)にのみ`b.fill()`を呼び出します。これにより、`fill()`がバッファ満杯の状態で呼び出されることがなくなり、`net/textproto`のテスト失敗が解決されます。コメント`// buffer is not full`が追加され、この前提が強調されています。
4. **`ReadRune()` メソッドの変更**:
* `ReadRune()`の`fill()`呼び出し条件に`&& b.w-b.r < len(b.buf)`が追加されました。これは、`ReadSlice`と同様に、`fill()`がバッファが満杯の状態で呼び出されないようにするためのガードです。
5. **`WriteTo()` メソッドの変更**:
* `WriteTo()`のループ条件が変更され、`b.fill()`の呼び出しタイミングが調整されました。以前はループ内で常に`b.fill()`を呼び出していましたが、新しいコードでは、バッファが空ではない場合にのみ`writeBuf`を呼び出し、その後`b.fill()`を呼び出すように変更されています。これにより、`fill()`がより意図されたタイミングで呼び出されるようになります。
6. **`writeBuf()` メソッドの変更**:
* `w.Write`が実際に書き込んだバイト数`n`が、書き込もうとしたバイト数(`b.r-b.w`、ただしこれは`b.w-b.r`の誤記の可能性が高い、または`b.buf[b.r:b.w]`の長さ)よりも少ない場合にパニックを発生させるチェックが追加されました。これは、`io.Writer`の契約違反を早期に検出するための堅牢性向上です。
7. **テストケースの追加 (`bufio_test.go`)**:
* `TestZeroReader`という新しいテストケースが追加されました。このテストは、`Read`メソッドが常に`0, nil`を返す`zeroReader`というカスタム`io.Reader`を定義し、`bufio.Reader.ReadByte()`が無限ループに陥らないことを検証します。具体的には、`ReadByte()`の呼び出しをゴルーチンで実行し、タイムアウトを設定することで、無限ループが発生しないことを確認しています。これにより、Issue #7745で報告された問題が実際に修正されたことを保証します。
これらの変更は、`bufio.Reader`の内部状態管理と基盤の`io.Reader`との相互作用をより厳密にし、予期せぬ動作(特に無限ループ)を防ぐためのものです。
## コアとなるコードの変更箇所
### `src/pkg/bufio/bufio.go`
```diff
--- a/src/pkg/bufio/bufio.go
+++ b/src/pkg/bufio/bufio.go
@@ -88,15 +88,26 @@ func (b *Reader) fill() {
b.r = 0
}
- // Read new data.
- n, err := b.rd.Read(b.buf[b.w:])
- if n < 0 {
- panic(errNegativeRead)
+ if b.w >= len(b.buf) {
+ panic("bufio: tried to fill full buffer")
}
- b.w += n
- if err != nil {
- b.err = err
+
+ // 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
}
func (b *Reader) readErr() error {
@@ -116,8 +127,9 @@ func (b *Reader) Peek(n int) ([]byte, error) {
if n > len(b.buf) {
return nil, ErrBufferFull
}
+ // 0 <= n <= len(b.buf)
for b.w-b.r < n && b.err == nil {
- b.fill()
+ b.fill() // b.w-b.r < len(b.buf) => buffer is not full
}
m := b.w - b.r
if m > n {
@@ -143,7 +155,7 @@ func (b *Reader) Read(p []byte) (n int, err error) {
if n == 0 {
return 0, b.readErr()
}
- if b.w == b.r {
+ if b.r == b.w {
if b.err != nil {
return 0, b.readErr()
}
@@ -151,13 +163,16 @@ func (b *Reader) Read(p []byte) (n int, err error) {
if len(p) >= len(b.buf) {
// 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
}
return n, b.readErr()
}
- b.fill()
+ b.fill() // buffer is empty
if b.w == b.r {
return 0, b.readErr()
}
@@ -181,7 +196,7 @@ func (b *Reader) ReadByte() (c byte, err error) {
if b.err != nil {
return 0, b.readErr()
}
- b.fill()
+ b.fill() // buffer is empty
}
c = b.buf[b.r]
b.r++
@@ -211,8 +226,8 @@ func (b *Reader) UnreadByte() error {
// rune and its size in bytes. If the encoded rune is invalid, it consumes one byte
// and returns unicode.ReplacementChar (U+FFFD) with a size of 1.
func (b *Reader) ReadRune() (r rune, size int, err error) {
- for b.r+utf8.UTFMax > b.w && !utf8.FullRune(b.buf[b.r:b.w]) && b.err == nil {
- b.fill()
+ for b.r+utf8.UTFMax > b.w && !utf8.FullRune(b.buf[b.r:b.w]) && b.err == nil && b.w-b.r < len(b.buf) {
+ b.fill() // b.w-b.r < len(buf) => buffer is not full
}
b.lastRuneSize = -1
if b.r == b.w {
@@ -256,36 +271,28 @@ func (b *Reader) Buffered() int { return b.w - b.r }
// ReadBytes or ReadString instead.
// ReadSlice returns err != nil if and only if line does not end in delim.
func (b *Reader) ReadSlice(delim byte) (line []byte, err error) {
- // Look in buffer.
- if i := bytes.IndexByte(b.buf[b.r:b.w], delim); i >= 0 {
- line1 := b.buf[b.r : b.r+i+1]
- b.r += i + 1
- return line1, nil
- }
-
- // Read more into buffer, until buffer fills or we find delim.
for {
+ // Search buffer.
+ if i := bytes.IndexByte(b.buf[b.r:b.w], delim); i >= 0 {
+ line := b.buf[b.r : b.r+i+1]
+ b.r += i + 1
+ return line, nil
+ }
+
+ // Pending error?
if b.err != nil {
line := b.buf[b.r:b.w]
b.r = b.w
return line, b.readErr()
}
- n := b.Buffered()
- b.fill()
-
- // Search new part of buffer
- if i := bytes.IndexByte(b.buf[n:b.w], delim); i >= 0 {
- line := b.buf[0 : n+i+1]
- b.r = n + i + 1
- return line, nil
- }
-
- // Buffer is full?
- if b.Buffered() >= len(b.buf) {
+ // Buffer full?
+ if n := b.Buffered(); n >= len(b.buf) {
b.r = b.w
return b.buf, ErrBufferFull
}
+
+ b.fill() // buffer is not full
}
}
@@ -417,12 +424,18 @@ func (b *Reader) WriteTo(w io.Writer) (n int64, err error) {
return n, err
}
- for b.fill(); b.r < b.w; b.fill() {
+ if b.w-b.r < len(b.buf) {
+ b.fill() // buffer not full
+ }
+
+ for b.r < b.w {
+ // b.r < b.w => buffer is not empty
m, err := b.writeBuf(w)
n += m
if err != nil {
return n, err
}
+ b.fill() // buffer is empty
}
if b.err == io.EOF {
@@ -435,6 +448,9 @@ func (b *Reader) WriteBuf(w io.Writer) (int64, error) {
// writeBuf writes the Reader's buffer to the writer.
func (b *Reader) writeBuf(w io.Writer) (int64, error) {
n, err := w.Write(b.buf[b.r:b.w])
+ if n < b.r-b.w {
+ panic(errors.New("bufio: writer did not write all data"))
+ }
b.r += n
return int64(n), err
}
src/pkg/bufio/bufio_test.go
--- a/src/pkg/bufio/bufio_test.go
+++ b/src/pkg/bufio/bufio_test.go
@@ -14,6 +14,7 @@ import (
"strings"
"testing"
"testing/iotest"
+ "time"
"unicode/utf8"
)
@@ -174,6 +175,34 @@ 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
-
fill()メソッド:if b.w >= len(b.buf) { panic("bufio: tried to fill full buffer") }: この行は、fill()が呼び出される前にバッファが満杯でないことを保証するためのガードです。もしバッファが満杯の状態でfill()が呼び出された場合、これはプログラミングエラーと見なされ、パニックが発生します。これにより、ReadSliceのような呼び出し元がfill()を不適切に呼び出すことを防ぎ、net/textprotoのテスト失敗の原因となっていた問題を解決します。for i := maxConsecutiveEmptyReads; i > 0; i--: ここが無限ループを防ぐための主要な変更点です。maxConsecutiveEmptyReadsは、基盤のio.Readerが0, nilを連続して返すことを許容する最大回数を定義します。ループ内でb.rd.Read(b.buf[b.w:])を呼び出し、以下の条件でループを終了します。n < 0: 負のバイト数が返された場合(不正なリーダーの動作)、パニック。err != nil: エラーが発生した場合、b.errにエラーを設定してreturn。n > 0: データが読み込まれた場合、return。
b.err = io.ErrNoProgress: ループがmaxConsecutiveEmptyReads回繰り返され、その間ずっと0, nilが返され続けた場合、つまり基盤のリーダーが進捗しないと判断された場合、b.errにio.ErrNoProgressを設定します。これにより、bufio.Readerの他のメソッドがこの状態を検出し、無限ループを回避できます。
-
ReadByte()メソッド:b.fill() // buffer is empty:ReadByteがバッファから読み取れない場合にfill()を呼び出す箇所です。fill()の変更により、ここで無限ループに陥る可能性がなくなりました。
-
ReadRune()メソッド:for b.r+utf8.UTFMax > b.w && !utf8.FullRune(b.buf[b.r:b.w]) && b.err == nil && b.w-b.r < len(b.buf):fill()を呼び出す条件に&& b.w-b.r < len(b.buf)が追加されました。これは、ReadRuneがfill()を呼び出す際に、バッファが満杯でないことを保証するためのものです。これにより、fill()がバッファ満杯の状態で呼び出されることを防ぎます。
-
ReadSlice()メソッド:- このメソッドは大幅に簡素化されました。以前はバッファの検索と
fill()の呼び出しが複雑に絡み合っていましたが、新しい実装では無限ループ内で以下のロジックを明確に実行します。if i := bytes.IndexByte(b.buf[b.r:b.w], delim); i >= 0: まず、現在のバッファ内で区切り文字を検索します。見つかれば、その部分を返し、b.rを更新します。if b.err != nil: 保留中のエラーがあれば、それを処理して返します。if n := b.Buffered(); n >= len(b.buf): 重要な変更点。バッファが満杯であるかどうかをチェックします。もし満杯であれば、ErrBufferFullを返します。これにより、ReadSliceがバッファの容量を超えて読み取ろうとすることを防ぎます。b.fill() // buffer is not full: 上記のチェックを通過した場合(つまり、バッファが満杯ではない場合)にのみfill()を呼び出します。これにより、fill()がバッファ満杯の状態で呼び出されることがなくなり、net/textprotoのテスト失敗が解決されます。
- このメソッドは大幅に簡素化されました。以前はバッファの検索と
-
WriteTo()メソッド:if b.w-b.r < len(b.buf) { b.fill() // buffer not full }:WriteToの開始時に、バッファに空きがある場合に一度fill()を呼び出すように変更されました。for b.r < b.w { ... b.fill() // buffer is empty }: ループ内で、バッファが空ではない場合にwriteBufを呼び出し、その後b.fill()を呼び出すように変更されました。これにより、fill()の呼び出しがより制御され、バッファが空になった後にのみ新しいデータを読み込むようになります。
-
writeBuf()メソッド:if n < b.r-b.w { panic(errors.New("bufio: writer did not write all data")) }:w.Writeが書き込もうとしたデータ量よりも少ないバイト数を返した場合にパニックを発生させるチェックが追加されました。これは、io.Writerの契約(要求されたバイト数を書き込むか、エラーを返す)に違反する実装を早期に検出するための堅牢性向上です。
src/pkg/bufio/bufio_test.go
TestZeroReader関数:type zeroReader struct{}:Readメソッドが常に0, nilを返すカスタムio.Readerを定義しています。func (zeroReader) Read(p []byte) (int, error) { return 0, nil }: このReadメソッドの実装が、Issue #7745で問題となった「進捗のないリーダー」の挙動を模倣しています。go func() { _, err := r.ReadByte(); c <- err }():bufio.ReaderのReadByte()メソッドを別のゴルーチンで実行します。select { ... case <-time.After(time.Second): t.Error("test timed out (endless loop in ReadByte?)") }:ReadByte()の呼び出しが1秒以内に完了し、io.ErrNoProgressエラーを返すことを期待しています。もし1秒を超えても完了しない場合、それは無限ループに陥っていることを意味し、テストは失敗します。これにより、ReadByteの無限ループ問題が修正されたことを自動的に検証できます。
これらの変更は、bufioパッケージの堅牢性を大幅に向上させ、特に不正なio.Readerの実装や特定のバッファリングシナリオにおける予期せぬ動作を防ぐことに貢献しています。
関連リンク
- Go Issue #7745: https://github.com/golang/go/issues/7745
- Go CL 86590043: https://golang.org/cl/86590043 (このコミットに対応するGoのコードレビューリンク)
参考にした情報源リンク
- Go言語
bufioパッケージのドキュメント: https://pkg.go.dev/bufio - Go言語
ioパッケージのドキュメント: https://pkg.go.dev/io io.ErrNoProgressに関するGoのドキュメント: https://pkg.go.dev/io#pkg-variables- Stack Overflow: "When does bufio.Reader return io.ErrNoProgress?": https://stackoverflow.com/questions/24073342/when-does-bufio-reader-return-io-errnoprogress
- Go言語の
io.Readerインターフェースの契約に関する議論: https://github.com/golang/go/issues/1993