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.

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

bufio: Reader swallows 0-length reads #17059

Closed
droyo opened this issue Sep 11, 2016 · 6 comments
Closed

bufio: Reader swallows 0-length reads #17059

droyo opened this issue Sep 11, 2016 · 6 comments
Labels
FrozenDueToAge NeedsFix The path to resolution is known, but the work has not been done.
Milestone

Comments

@droyo
Copy link

droyo commented Sep 11, 2016

I have a package that uses bufio.Reader to decode protocol messages. It implements
trace debugging by writing all messages through an io.Pipe(). I was surprised to find
that the first 100 0-length writes were ignored by the bufio.Reader.fill method, as
the documentation explicitly states

The bytes are taken from at most one Read on the underlying Reader, hence
n may be less than len(p).

I would like to suggest a documentation change to indicate that zero-length reads are
not counted.

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

go version go1.7 linux/amd64

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

GOARCH="amd64"
GOBIN="/usr/local/bin"
GOEXE=""
GOHOSTARCH="amd64"
GOHOSTOS="linux"
GOOS="linux"
GOPATH="/"
GORACE=""
GOROOT="/go"
GOTOOLDIR="/go/pkg/tool/linux_amd64"
CC="gcc"
GOGCCFLAGS="-fPIC -m64 -pthread -fmessage-length=0 -fdebug-prefix-map=/tmp/go-build706953720=/tmp/go-build -gno-record-gcc-switches"
CXX="g++"
CGO_ENABLED="1"

What did you do?

A zero-length write to an io.PipeWriter, whose corresponding
io.PipeReader was read through a bufio.Reader. The following
simplified program illustrates the issue: https://play.golang.org/p/lI14R5CMQ-

What did you expect to see?

The program should print one item for every write to the pipe:

1 foo
2 bar
3 
4 
5 
6 baz

What did you see instead?

The bufio.Reader ignored the 0-byte reads.

1 foo
2 bar
3 baz
@bradfitz
Copy link
Contributor

Without looking into the details of this issue just yet, I'll note that 0-byte reads and writes are inconsistent and ill-defined everywhere in Go.

@ianlancetaylor ianlancetaylor changed the title bufio.Reader swallows 0-length reads bufio: Reader swallows 0-length reads Sep 11, 2016
@ianlancetaylor
Copy link
Contributor

Any change to bufio in this regard needs to pay attention to #7611.

@ianlancetaylor ianlancetaylor added this to the Go1.8 milestone Sep 11, 2016
@droyo
Copy link
Author

droyo commented Sep 11, 2016

The issue is easy enough to work around if it is documented. I am curious about the original intent behind this behavior.

@ianlancetaylor
Copy link
Contributor

@droyo See issue #7611 and the CL that was committed to fix it. That CL may have been erroneous, but I think that is the intent behind the current behavior.

droyo added a commit to droyo/styx that referenced this issue Sep 12, 2016
Previously every write by an Encoder was written to the underlying
connection immediately. This can result in high CPU usage and poor
performance on high-latency connections. More importantly, it works
around golang/go#17059 .
@quentinmit quentinmit added the NeedsDecision Feedback is required from experts, contributors, and/or the community before a change can be made. label Oct 5, 2016
@rsc
Copy link
Contributor

rsc commented Oct 18, 2016

I think this is a bug. The fix in CL 86590043 for ReadByte (for #7745) incorrectly affected Read as well. Will send a CL.

@ianlancetaylor, #7611 is about bufio.Writer, not bufio.Reader, so I think these are mostly independent.

@rsc rsc self-assigned this Oct 18, 2016
@rsc rsc added NeedsFix The path to resolution is known, but the work has not been done. and removed NeedsDecision Feedback is required from experts, contributors, and/or the community before a change can be made. labels Oct 18, 2016
@gopherbot
Copy link

CL https://golang.org/cl/31320 mentions this issue.

@golang golang locked and limited conversation to collaborators Oct 18, 2017
@rsc rsc removed their assignment Jun 23, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
FrozenDueToAge NeedsFix The path to resolution is known, but the work has not been done.
Projects
None yet
Development

No branches or pull requests

6 participants