syscall: NewCallback triggers data race on Windows when used from different goroutine #50249

JohanKnutzen · 2021-12-17T20:15:24Z

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

$ go version
go version go1.17.4 windows/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
set GO111MODULE=
set GOARCH=amd64
set GOBIN=
set GOCACHE=C:\Users\knutz\AppData\Local\go-build
set GOENV=C:\Users\knutz\AppData\Roaming\go\env
set GOEXE=.exe
set GOEXPERIMENT=
set GOFLAGS=
set GOHOSTARCH=amd64
set GOHOSTOS=windows
set GOINSECURE=
set GOMODCACHE=C:\Users\knutz\go\pkg\mod
set GONOPROXY=
set GONOSUMDB=
set GOOS=windows
set GOPATH=C:\Users\knutz\go
set GOPRIVATE=
set GOPROXY=https://proxy.golang.org,direct
set GOROOT=C:\Program Files\Go
set GOSUMDB=sum.golang.org
set GOTMPDIR=
set GOTOOLDIR=C:\Program Files\Go\pkg\tool\windows_amd64
set GOVCS=
set GOVERSION=go1.17.4
set GCCGO=gccgo
set AR=ar
set CC=gcc
set CXX=g++
set CGO_ENABLED=1
set GOMOD=NUL
set CGO_CFLAGS=-g -O2
set CGO_CPPFLAGS=
set CGO_CXXFLAGS=-g -O2
set CGO_FFLAGS=-g -O2
set CGO_LDFLAGS=-g -O2
set PKG_CONFIG=pkg-config
set GOGCCFLAGS=-m64 -mthreads -fno-caret-diagnostics -Qunused-arguments -fmessage-length=0 -fdebug-prefix-map=C:\Users\knutz\AppData\Local\Temp\go-build2788134738=/tmp/go-build -gno-record-gcc-switches

What did you do?

package main

import (
	"syscall"
)

func foo() {
	syscall.NewCallback(func(callerContext uintptr, iface uintptr, notificationType uint32) uintptr {
		return 0
	})
}

func main() {
	go foo()
	foo()
}

Built and run using:
go build -race && ./main.exe

What did you expect to see?

Nothing

What did you see instead?

WARNING: DATA RACE
Read at 0x00c000028000 by goroutine 6:
  runtime.mapaccess2()
      C:/Program Files/Go/src/runtime/map.go:452 +0x0
  syscall.compileCallback()
      C:/Program Files/Go/src/runtime/syscall_windows.go:308 +0x244

Previous write at 0x00c000028000 by main goroutine:
  runtime.mapassign()
      C:/Program Files/Go/src/runtime/map.go:571 +0x0
  syscall.compileCallback()
      C:/Program Files/Go/src/runtime/syscall_windows.go:324 +0x392
  main.main()
      C:/code/fd/hack/compile_syscall_race/main.go:15 +0x30

Goroutine 6 (running) created at:
  main.main()
      C:/code/fd/hack/compile_syscall_race/main.go:14 +0x2b
==================
Found 1 data race(s)```

The text was updated successfully, but these errors were encountered:

randall77 · 2021-12-17T23:16:21Z

Those map accesses are guarded by a lock. Not sure why the race detector would report on them.

JohanKnutzen · 2021-12-18T07:28:28Z

@randall77 From reviewing the code, that seems to be the case. Perhaps this is a bug in the race detector? More specifically when run on Windows

cherrymui · 2021-12-21T16:07:30Z

The race detector probably doesn't know about the lock. It seems runtime lock does not inform the race detector about the edge (unlike locks in the sync package). (Normally it is fine as the runtime is not race-instrumented. But that code uses a map and mapassign/mapaccess have explicit race calls).

gopherbot · 2022-06-28T21:42:56Z

Change https://go.dev/cl/414518 mentions this issue: runtime: add race annotations to cbs.lock

prattmic · 2022-06-29T20:13:57Z

@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 · 2022-06-29T20:17:54Z

Backport issue(s) opened: #53612 (for 1.17), #53613 (for 1.18).

Remember to create the cherry-pick CL(s) as soon as the patch is submitted to master, according to https://go.dev/wiki/MinorReleases.

gopherbot · 2022-06-29T20:20:40Z

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

gopherbot · 2022-06-29T20:21:17Z

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

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>

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 #53612. 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/+/415197

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>

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#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>

dmitshur added NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. OS-Windows labels Dec 21, 2021

cherrymui mentioned this issue Jun 28, 2022

runtime/metrics: data race detected in Read #53542

Closed

prattmic self-assigned this Jun 28, 2022

prattmic added this to the Go1.19 milestone Jun 28, 2022

prattmic added NeedsFix The path to resolution is known, but the work has not been done. and removed NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. labels Jun 28, 2022

gopherbot closed this as completed in 20760cf Jun 29, 2022

This was referenced Jun 29, 2022

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

Closed

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

Closed

golang locked and limited conversation to collaborators Jun 29, 2023

gopherbot added the FrozenDueToAge label Jun 29, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

syscall: NewCallback triggers data race on Windows when used from different goroutine #50249

syscall: NewCallback triggers data race on Windows when used from different goroutine #50249

JohanKnutzen commented Dec 17, 2021

randall77 commented Dec 17, 2021

JohanKnutzen commented Dec 18, 2021

cherrymui commented Dec 21, 2021

gopherbot commented Jun 28, 2022

prattmic commented Jun 29, 2022

gopherbot commented Jun 29, 2022

gopherbot commented Jun 29, 2022

gopherbot commented Jun 29, 2022

Navigation Menu

syscall: NewCallback triggers data race on Windows when used from different goroutine #50249

syscall: NewCallback triggers data race on Windows when used from different goroutine #50249

Comments

JohanKnutzen commented Dec 17, 2021

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

Does this issue reproduce with the latest release?

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

What did you do?

What did you expect to see?

What did you see instead?

randall77 commented Dec 17, 2021

JohanKnutzen commented Dec 18, 2021

cherrymui commented Dec 21, 2021

gopherbot commented Jun 28, 2022

prattmic commented Jun 29, 2022

gopherbot commented Jun 29, 2022

gopherbot commented Jun 29, 2022

gopherbot commented Jun 29, 2022

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

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