[インデックス 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