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/go, testing: race detector finding a data race doesn't fail a test #15972

Closed
stevekuznetsov opened this issue Jun 6, 2016 · 4 comments

Comments

Projects
None yet
6 participants
@stevekuznetsov
Copy link

commented Jun 6, 2016

Environment details:

$ go version
go version go1.6.2 linux/amd64
$ go env
GOARCH="amd64"
GOBIN=""
GOEXE=""
GOHOSTARCH="amd64"
GOHOSTOS="linux"
GOOS="linux"
GOPATH="/home/skuznets/Documents/go"
GORACE=""
GOROOT="/home/skuznets/.gvm/gos/go1.6.2"
GOTOOLDIR="/home/skuznets/.gvm/gos/go1.6.2/pkg/tool/linux_amd64"
GO15VENDOREXPERIMENT="1"
CC="gcc"
GOGCCFLAGS="-fPIC -m64 -pthread -fmessage-length=0"
CXX="g++"
CGO_ENABLED="1"

What did you do?

Run a unit test with a data race using the following invocation:

go test  -v -race -cover -covermode atomic -timeout 120s github.com/openshift/origin/pkg/service/controller/servingcert

What did you expect to see?

When the race detector is enabled and a data race is found, the unit test will fail.

What did you see instead?

The test passes, but the package fails:

=== RUN   TestSecretCreationErrorControllerFlow
I0606 13:37:58.051502   25372 controller.go:132] Shutting down service signing cert controller
==================
WARNING: DATA RACE
Write by goroutine 54:
  runtime.mapassign1()
      /home/skuznets/.gvm/gos/go1.6.2/src/runtime/hashmap.go:429 +0x0
  github.com/openshift/origin/pkg/service/controller/servingcert.(*ServiceServingCertController).syncService()
      github.com/openshift/origin/pkg/service/controller/servingcert/_test/_obj_test/controller.go:288 +0x21af
  github.com/openshift/origin/pkg/service/controller/servingcert.TestAlreadyExistingSecretForDifferentUIDControllerFlow.func2()
      /home/skuznets/Documents/go/src/github.com/openshift/origin/pkg/service/controller/servingcert/controller_test.go:234 +0xc2
  github.com/openshift/origin/pkg/service/controller/servingcert.(*ServiceServingCertController).worker_inner()
      github.com/openshift/origin/pkg/service/controller/servingcert/_test/_obj_test/controller.go:173 +0x20e
  github.com/openshift/origin/pkg/service/controller/servingcert.(*ServiceServingCertController).worker()
      github.com/openshift/origin/pkg/service/controller/servingcert/_test/_obj_test/controller.go:155 +0x6a
  github.com/openshift/origin/pkg/service/controller/servingcert.(*ServiceServingCertController).(github.com/openshift/origin/pkg/service/controller/servingcert.worker)-fm()
      github.com/openshift/origin/pkg/service/controller/servingcert/_test/_obj_test/controller.go:127 +0x2d
  k8s.io/kubernetes/pkg/util/wait.JitterUntil.func1()
      /home/skuznets/Documents/go/src/github.com/openshift/origin/Godeps/_workspace/src/k8s.io/kubernetes/pkg/util/wait/wait.go:66 +0x58
  k8s.io/kubernetes/pkg/util/wait.JitterUntil()
      /home/skuznets/Documents/go/src/github.com/openshift/origin/Godeps/_workspace/src/k8s.io/kubernetes/pkg/util/wait/wait.go:67 +0x7d
  k8s.io/kubernetes/pkg/util/wait.Until()
      /home/skuznets/Documents/go/src/github.com/openshift/origin/Godeps/_workspace/src/k8s.io/kubernetes/pkg/util/wait/wait.go:47 +0x4b

Previous read by goroutine 50:
  reflect.maplen()
      /home/skuznets/.gvm/gos/go1.6.2/src/runtime/hashmap.go:1029 +0x0
  reflect.Value.Len()
      /home/skuznets/.gvm/gos/go1.6.2/src/reflect/value.go:1007 +0x150
  reflect.deepValueEqual()
      /home/skuznets/.gvm/gos/go1.6.2/src/reflect/deepequal.go:106 +0xa14
  reflect.DeepEqual()
      /home/skuznets/.gvm/gos/go1.6.2/src/reflect/deepequal.go:185 +0x263
  github.com/openshift/origin/pkg/service/controller/servingcert.TestAlreadyExistingSecretForDifferentUIDControllerFlow()
      /home/skuznets/Documents/go/src/github.com/openshift/origin/pkg/service/controller/servingcert/controller_test.go:269 +0x163b
  testing.tRunner()
      /home/skuznets/.gvm/gos/go1.6.2/src/testing/testing.go:473 +0xdc

Goroutine 54 (running) created at:
  github.com/openshift/origin/pkg/service/controller/servingcert.(*ServiceServingCertController).Run()
      github.com/openshift/origin/pkg/service/controller/servingcert/_test/_obj_test/controller.go:127 +0x180

Goroutine 50 (finished) created at:
  testing.RunTests()
      /home/skuznets/.gvm/gos/go1.6.2/src/testing/testing.go:582 +0xae2
  testing.(*M).Run()
      /home/skuznets/.gvm/gos/go1.6.2/src/testing/testing.go:515 +0x11d
  main.main()
      github.com/openshift/origin/pkg/service/controller/servingcert/_test/_testmain.go:108 +0x385
==================
--- PASS: TestSecretCreationErrorControllerFlow (13.63s)
coverage: 81.2% of statements
Found 1 data race(s)
FAIL    github.com/openshift/origin/pkg/service/controller/servingcert  37.923s

@stevekuznetsov stevekuznetsov changed the title test: race detctor finding a data race doesn't fail a test test: race detector finding a data race doesn't fail a test Jun 6, 2016

@ianlancetaylor ianlancetaylor changed the title test: race detector finding a data race doesn't fail a test cmd/go, testing: race detector finding a data race doesn't fail a test Jun 6, 2016

@ianlancetaylor ianlancetaylor added this to the Go1.8 milestone Jun 6, 2016

@quentinmit quentinmit added the NeedsFix label Oct 6, 2016

@rsc

This comment has been minimized.

Copy link
Contributor

commented Oct 21, 2016

It's true: we don't know which test case has the race. We just know that the overall test binary failed. In general test cases can run in parallel or have their background goroutines keep running after the test case is ostensibly over. So assigning races to specific test cases is harder than it sounds. Even so, if the race detector exposed the count of problems found so far, we could at least check to see if it increased during a particular test case.

@dvyukov, is it easy for Go to fetch the number of reported problems out of the race detector library on demand? Thanks.

llvm-mirror pushed a commit to llvm-mirror/compiler-rt that referenced this issue Oct 28, 2016

tsan: add a hook to obtain number of reports
Requested in:
golang/go#15972
Will help to fail individual test cases with races.



git-svn-id: https://llvm.org/svn/llvm-project/compiler-rt/trunk@285455 91177308-0d34-0410-b5e6-96231b3b80d8

chapuni pushed a commit to llvm-project/llvm-project that referenced this issue Oct 28, 2016

tsan: add a hook to obtain number of reports
Requested in:
golang/go#15972
Will help to fail individual test cases with races.

chapuni pushed a commit to llvm-project/llvm-project-submodule that referenced this issue Oct 28, 2016

tsan: add a hook to obtain number of reports
Requested in:
golang/go#15972
Will help to fail individual test cases with races.
@gopherbot

This comment has been minimized.

Copy link

commented Oct 28, 2016

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

gopherbot pushed a commit that referenced this issue Oct 30, 2016

runtime/race: update race runtime
This updates the runtime to HEAD to keep it aligned and fixes some bugs.

http://llvm.org/viewvc/llvm-project?view=revision&revision=285454
fixes the crash on darwin related to unaligned data section (#17065).

http://llvm.org/viewvc/llvm-project?view=revision&revision=285451
enables core dumps by default (#16527).

http://llvm.org/viewvc/llvm-project?view=revision&revision=285455
adds a hook to obtain number of races reported so far (#15972).
Can now be obtained with:

//go:nosplit
func RaceReportCount() int {
	var n uint64
	racecall(&__tsan_report_count, uintptr(unsafe.Pointer(&n)), 0, 0, 0)
	return int(n)
}

Fixes #16527.
Fixes #17065.
Update #15972.

Change-Id: I8f869cb6275c9521a47303f3810a9965e9314357
Reviewed-on: https://go-review.googlesource.com/32160
Run-TryBot: Dmitry Vyukov <dvyukov@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
@dvyukov

This comment has been minimized.

Copy link
Member

commented Oct 30, 2016

The updated race runtime exposes number of found races. It can be obtained with:

//go:nosplit
func RaceReportCount() int {
    var n uint64
    racecall(&__tsan_report_count, uintptr(unsafe.Pointer(&n)), 0, 0, 0)
    return int(n)
}

llvm-beanz added a commit to llvm-beanz/llvm-submodules that referenced this issue Nov 1, 2016

tsan: add a hook to obtain number of reports
Requested in:
golang/go#15972
Will help to fail individual test cases with races.

git-svn-id: https://llvm.org/svn/llvm-project/compiler-rt/trunk@285455 91177308-0d34-0410-b5e6-96231b3b80d8
@gopherbot

This comment has been minimized.

Copy link

commented Nov 3, 2016

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

@gopherbot gopherbot closed this in 43f954e Nov 3, 2016

@golang golang locked and limited conversation to collaborators Nov 3, 2017

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
You can’t perform that action at this time.