Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

bufio: Reader.Read can return a non-zero count and io.EOF #52577

Closed
DeedleFake opened this issue Apr 26, 2022 · 7 comments
Closed

bufio: Reader.Read can return a non-zero count and io.EOF #52577

DeedleFake opened this issue Apr 26, 2022 · 7 comments
Labels
FrozenDueToAge NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Milestone
Backlog

Comments

@DeedleFake
Copy link

What version of Go are you using (go version)?

$ go version
go version go1.18.1 linux/amd64

Does this issue reproduce with the latest release?

Yes.

What operating system and processor architecture are you using (go env)?

go env Output
$ go env
GO111MODULE=""
GOARCH="amd64"
GOBIN=""
GOCACHE="$HOME/.cache/go-build"
GOENV="$HOME/.config/go/env"
GOEXE=""
GOEXPERIMENT=""
GOFLAGS=""
GOHOSTARCH="amd64"
GOHOSTOS="linux"
GOINSECURE=""
GOMODCACHE="$HOME/go/pkg/mod"
GONOPROXY=""
GONOSUMDB=""
GOOS="linux"
GOPATH="$HOME/go"
GOPRIVATE=""
GOPROXY="https://proxy.golang.org,direct"
GOROOT="/usr/lib/go"
GOSUMDB="sum.golang.org"
GOTMPDIR=""
GOTOOLDIR="/usr/lib/go/pkg/tool/linux_amd64"
GOVCS=""
GOVERSION="go1.18.1"
GCCGO="gccgo"
GOAMD64="v1"
AR="ar"
CC="gcc"
CXX="g++"
CGO_ENABLED="1"
GOMOD="/dev/null"
GOWORK=""
CGO_CFLAGS="-g -O2"
CGO_CPPFLAGS=""
CGO_CXXFLAGS="-g -O2"
CGO_FFLAGS="-g -O2"
CGO_LDFLAGS="-g -O2"
PKG_CONFIG="pkg-config"
GOGCCFLAGS="-fPIC -m64 -pthread -fmessage-length=0 -fdebug-prefix-map=/tmp/go-build2342447434=/tmp/go-build -gno-record-gcc-switches"

What did you do?

Playground Link

In the comments of a recent post to Reddit, it was noted that under very specific circumstances, bufio.Reader.Read could return a non-zero count and io.EOF simultaneously. This is despite the docs saying

At EOF, the count will be zero and err will be io.EOF.

This can be read to mean either that it never returns a non-zero count and io.EOF, but it can also be read to mean that it will always return a zero count when the underlying reader hits EOF. I'm not entirely sure which is meant, but if it's the first then the code doesn't do what the documentation says, and if it's the latter then it should probably be clarified.

What did you expect to see?

14, <nil> -> "this is a test"
0, EOF -> ""

What did you see instead?

14, EOF -> "this is a test"
0, EOF -> ""
@dsnet
Copy link
Member

dsnet commented Apr 26, 2022

I think the bufio docs just need to be updated.

The behavior of whether bufio.Reader returns early io.EOF should be a reflection of whether the underlying io.Reader returns an early io.EOF or not. Some code out there depends on early io.EOF as an optimization to signal that the underlying io.Reader is fully consumed.

@bcmills
Copy link
Contributor

bcmills commented Apr 28, 2022

Or, why not fix the implementation to match the comment? The comment has been there ~forever, and the behavior it describes — while not strictly required by the io.Reader interface — doesn't violate it either.

@dsnet
Copy link
Member

dsnet commented Apr 28, 2022

@bcmills, per my earlier comment, there is a performance implication to removing the early EOF return.

@DeedleFake
Copy link
Author

Could you provide an example of such a reliance specifically with a bufio.Reader, @dsnet?

@dsnet
Copy link
Member

dsnet commented Apr 28, 2022 •
edited

You can take a look at https://golang.org/cl/21302 and all the referenced CLs and issues. This is one that I can remember, but I know there are other cases.

In general, if you read a stream of records of some known length N, the early EOF lets you know that there are no more records incoming and allows you to either return earlier with the results and/or reuse the resources sooner.

@cagedmantis cagedmantis added the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label May 2, 2022
@cagedmantis cagedmantis added this to the Backlog milestone May 2, 2022
@cagedmantis
Copy link
Contributor

/cc @griesemer @bradfitz @ianlancetaylor

@gopherbot
Copy link

Change https://go.dev/cl/403594 mentions this issue: bufio: clarify io.EOF behavior of Reader.Read

@golang golang locked and limited conversation to collaborators May 2, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
FrozenDueToAge NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Projects
None yet
Milestone
Backlog
Development

No branches or pull requests
5 participants
@cagedmantis @DeedleFake @bcmills @dsnet @gopherbot