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

Jump to bottom

syscall: NewCallback triggers data race on Windows when used from different goroutine [1.18 backport] #53613

Closed
gopherbot opened this issue Jun 29, 2022 · 3 comments
Closed

syscall: NewCallback triggers data race on Windows when used from different goroutine [1.18 backport] #53613

gopherbot opened this issue Jun 29, 2022 · 3 comments
Labels
CherryPickApproved Used during the release process for point releases FrozenDueToAge
Milestone
Go1.18.4

Comments

@gopherbot
Copy link

@prattmic requested issue #50249 to be considered for backport to the next 1.18 minor release.

@gopherbot please backport to 1.17 and 1.18. This triggers a false positive in the race detector for programs calling syscall.NewCallback from multiple goroutines. There is a workaround: use a single global lock around all calls to syscall.NewCallback in the program. However, that is quite invasive and difficult for complex code.
@gopherbot gopherbot added the CherryPickCandidate Used during the release process for point releases label Jun 29, 2022
@gopherbot gopherbot mentioned this issue Jun 29, 2022
Closed
@gopherbot gopherbot added this to the Go1.18.4 milestone Jun 29, 2022
@gopherbot
Copy link
Author

Change https://go.dev/cl/415198 mentions this issue: [release-branch.go1.18] runtime: add race annotations to cbs.lock

@dmitshur
Copy link
Contributor

The rationale to backport and safety of fix here seems very similar to #53590, so it can possibly be handled in a similar way.

@joedian joedian added the CherryPickApproved Used during the release process for point releases label Jul 6, 2022
@gopherbot gopherbot removed the CherryPickCandidate Used during the release process for point releases label Jul 6, 2022
@gopherbot
Copy link
Author

Closed by merging e5ca37d to release-branch.go1.18.

gopherbot pushed a commit that referenced this issue Jul 6, 2022
@prattmic @heschi
[release-branch.go1.18] runtime: add race annotations to cbs.lock
e5ca37d 
cbs.lock protects a map. The map implementation is race instrumented
regardless of which package is it called from.

lock/unlock are not automatically race instrumented, so we can trigger
race false positives without manually annotating our lock acquire and
release.

compileCallback is used during initialization before the P is available,
at which point raceacquire will crash during a racecallback to get the
race proc. Thus we skip instrumentation until scheduler initialization
is complete.

Fixes #53613.
For #50249.

Change-Id: Ie49227c9e9210ffbf0aee65f86f2b7b6a2f64638
Reviewed-on: https://go-review.googlesource.com/c/go/+/414518
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Cherry Mui <cherryyz@google.com>
Run-TryBot: Michael Pratt <mpratt@google.com>
(cherry picked from commit 20760cf)
Reviewed-on: https://go-review.googlesource.com/c/go/+/415198
Run-TryBot: Heschi Kreinick <heschi@google.com>
bradfitz pushed a commit to tailscale/go that referenced this issue Jul 14, 2022
@prattmic @bradfitz
[release-branch.go1.18] runtime: add race annotations to cbs.lock
36e2b1f 
cbs.lock protects a map. The map implementation is race instrumented
regardless of which package is it called from.

lock/unlock are not automatically race instrumented, so we can trigger
race false positives without manually annotating our lock acquire and
release.

compileCallback is used during initialization before the P is available,
at which point raceacquire will crash during a racecallback to get the
race proc. Thus we skip instrumentation until scheduler initialization
is complete.

Fixes golang#53613.
For golang#50249.

Change-Id: Ie49227c9e9210ffbf0aee65f86f2b7b6a2f64638
Reviewed-on: https://go-review.googlesource.com/c/go/+/414518
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Cherry Mui <cherryyz@google.com>
Run-TryBot: Michael Pratt <mpratt@google.com>
(cherry picked from commit 20760cf)
Reviewed-on: https://go-review.googlesource.com/c/go/+/415198
Run-TryBot: Heschi Kreinick <heschi@google.com>
@golang golang locked and limited conversation to collaborators Jul 6, 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
Projects
None yet
Milestone
Go1.18.4
Development

No branches or pull requests
3 participants
@dmitshur @gopherbot @joedian