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

cmd/go: data race in TestScript [1.18 backport] #54636

Closed
gopherbot opened this issue Aug 23, 2022 · 3 comments
Closed

cmd/go: data race in TestScript [1.18 backport] #54636

gopherbot opened this issue Aug 23, 2022 · 3 comments
Labels
CherryPickApproved Used during the release process for point releases FrozenDueToAge GoCommand cmd/go
Milestone
Go1.18.6

Comments

@gopherbot
Copy link
Contributor

@bcmills requested issue #54423 to be considered for backport to the next 1.18 minor release.

@gopherbot, please backport to go 1.18 and 1.19. This causes a data race in cmd/go tests.
@gopherbot gopherbot added the CherryPickCandidate Used during the release process for point releases label Aug 23, 2022
@gopherbot gopherbot mentioned this issue Aug 23, 2022
Closed
@gopherbot gopherbot added this to the Go1.18.6 milestone Aug 23, 2022
@bcmills
Copy link
Contributor

bcmills commented Aug 24, 2022

This is a data race in cmd/go tests, and the fix is low-risk.

@bcmills bcmills added CherryPickApproved Used during the release process for point releases GoCommand cmd/go labels Aug 24, 2022
@gopherbot gopherbot removed the CherryPickCandidate Used during the release process for point releases label Aug 24, 2022
@gopherbot
Copy link
Contributor Author

Change https://go.dev/cl/425208 mentions this issue: [release-branch.go1.18] cmd/go: avoid registering AtExit handlers in tests

@gopherbot
Copy link
Contributor Author

Closed by merging 7a575a5 to release-branch.go1.18.

gopherbot pushed a commit that referenced this issue Aug 29, 2022
@heschi
[release-branch.go1.18] cmd/go: avoid registering AtExit handlers in …
7a575a5 
…tests

Ever since 'go build' was added (in CL 5483069), it has used an atexit
handler to clean up working directories.

CL 154109 introduced 'cc' command to the script test framework that
called Init on a builder once per invocation. Unfortunately, since
base.AtExit is unsynchronized, the Init added there caused any script
that invokes that command to be unsafe for concurrent use.

This change fixes the race by having the 'cc' command pass in its
working directory instead of allowing the Builder to allocate one.
Following modern Go best practices, it also replaces the in-place Init
method (which is prone to typestate and aliasing bugs) with a
NewBuilder constructor function.

Updates #54423.
Fixes #54636.

Change-Id: I8fc2127a7d877bb39a1174e398736bb51d03d4d2
Reviewed-on: https://go-review.googlesource.com/c/go/+/425205
TryBot-Result: Gopher Robot <[email protected]>
Run-TryBot: Bryan Mills <[email protected]>
Auto-Submit: Bryan Mills <[email protected]>
Reviewed-by: Than McIntosh <[email protected]>
(cherry picked from commit d5aa088)
Reviewed-on: https://go-review.googlesource.com/c/go/+/425208
@golang golang locked and limited conversation to collaborators Aug 29, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
CherryPickApproved Used during the release process for point releases FrozenDueToAge GoCommand cmd/go
Projects
None yet
Milestone
Go1.18.6
Development

No branches or pull requests
2 participants
@bcmills @gopherbot