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

cmd/covdata: short read on string table when merging coverage counters [1.20 backport] #58427

Closed
gopherbot opened this issue Feb 9, 2023 · 4 comments
Labels
CherryPickApproved Used during the release process for point releases FrozenDueToAge
Milestone

Comments

@gopherbot
Copy link

@thanm requested issue #58411 to be considered for backport to the next 1.20 minor release.

@gopherbot please backport to 1.20

@gopherbot gopherbot added the CherryPickCandidate Used during the release process for point releases label Feb 9, 2023
@gopherbot gopherbot added this to the Go1.20.1 milestone Feb 9, 2023
@FALA00

This comment was marked as off-topic.

@FALA00

This comment was marked as spam.

@gopherbot gopherbot modified the milestones: Go1.20.1, Go1.20.2 Feb 14, 2023
@prattmic prattmic added the CherryPickApproved Used during the release process for point releases label Feb 15, 2023
@gopherbot gopherbot removed the CherryPickCandidate Used during the release process for point releases label Feb 15, 2023
@gopherbot
Copy link
Author

Change https://go.dev/cl/468536 mentions this issue: [release-branch.go1.20] cmd/internal/cov: fix misuse of bufio.Reader.Read in read helper

gopherbot pushed a commit that referenced this issue Feb 22, 2023
…Read in read helper

Fix a misuse of bufio.Reader.Read in the helper class
cmd/internal/cov.MReader; the MReader method in question should have
been using io.ReadFull (passing the bufio.Reader) instead of directly
calling Read.

Using the Read method instead of io.ReadFull will result in a "short"
read when processing a specific subset of counter data files, e.g.
those that are short enough to not trigger the mmap-based scheme we
use for larger files, but also with a large args section (something
large enough to exceed the default 4k buffer size used by
bufio.Reader).

Along the way, add some additional defered Close() calls for files
opened by the CovDataReader.visitPod, to enure we don't leave any open
file descriptor following a call to CovDataReader.Visit.

Fixes #58427.
Updates #58411.

Change-Id: Iea48dc25c0081be1ade29f3a633df02a681fd941
Reviewed-on: https://go-review.googlesource.com/c/go/+/466677
Run-TryBot: Than McIntosh <thanm@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: David Chase <drchase@google.com>
(cherry picked from commit a7fe9ad)
Reviewed-on: https://go-review.googlesource.com/c/go/+/468536
Reviewed-by: Cherry Mui <cherryyz@google.com>
@gopherbot
Copy link
Author

Closed by merging ac556f3 to release-branch.go1.20.

romaindoumenc pushed a commit to TroutSoftware/go that referenced this issue Mar 3, 2023
…Read in read helper

Fix a misuse of bufio.Reader.Read in the helper class
cmd/internal/cov.MReader; the MReader method in question should have
been using io.ReadFull (passing the bufio.Reader) instead of directly
calling Read.

Using the Read method instead of io.ReadFull will result in a "short"
read when processing a specific subset of counter data files, e.g.
those that are short enough to not trigger the mmap-based scheme we
use for larger files, but also with a large args section (something
large enough to exceed the default 4k buffer size used by
bufio.Reader).

Along the way, add some additional defered Close() calls for files
opened by the CovDataReader.visitPod, to enure we don't leave any open
file descriptor following a call to CovDataReader.Visit.

Fixes golang#58427.
Updates golang#58411.

Change-Id: Iea48dc25c0081be1ade29f3a633df02a681fd941
Reviewed-on: https://go-review.googlesource.com/c/go/+/466677
Run-TryBot: Than McIntosh <thanm@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: David Chase <drchase@google.com>
(cherry picked from commit a7fe9ad)
Reviewed-on: https://go-review.googlesource.com/c/go/+/468536
Reviewed-by: Cherry Mui <cherryyz@google.com>
@golang golang locked and limited conversation to collaborators Feb 22, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
CherryPickApproved Used during the release process for point releases FrozenDueToAge
Projects
None yet
Development

No branches or pull requests

3 participants