Skip to content

iter: incorrect race instrumentation is causing false positives in iter.Pull2 #64651

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
Closed
j178 opened this issue Dec 11, 2023 · 6 comments
Closed

iter: incorrect race instrumentation is causing false positives in iter.Pull2 #64651

j178 opened this issue Dec 11, 2023 · 6 comments
Assignees
@mauri870
Labels
FrozenDueToAge NeedsFix The path to resolution is known, but the work has not been done. okay-after-rc1 Used by release team to mark a release-blocker issue as okay to resolve either before or after rc1 RaceDetector release-blocker
Milestone
Go1.22

Comments

@j178
Copy link
Contributor

j178 commented Dec 11, 2023

Go version

go version devel go1.22-46ea4ab Sat Dec 9 21:48:06 2023 +0000 windows/amd64

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

set GO111MODULE=on
set GOARCH=amd64
set GOBIN=
set GOCACHE=C:\Users\nigel\AppData\Local\go-build
set GOENV=C:\Users\nigel\AppData\Roaming\go\env
set GOEXE=.exe
set GOEXPERIMENT=
set GOFLAGS=
set GOHOSTARCH=amd64
set GOHOSTOS=windows
set GOINSECURE=
set GOMODCACHE=C:\Users\nigel\go\pkg\mod
set GONOPROXY=
set GONOSUMDB=
set GOOS=windows
set GOPATH=C:\Users\nigel\go
set GOPRIVATE=
set GOPROXY=https://goproxy.cn,direct
set GOROOT=C:\Users\nigel\sdk\gotip
set GOSUMDB=sum.golang.org
set GOTMPDIR=
set GOTOOLCHAIN=auto
set GOTOOLDIR=C:\Users\nigel\sdk\gotip\pkg\tool\windows_amd64
set GOVCS=
set GOVERSION=devel go1.22-46ea4ab Sat Dec 9 21:48:06 2023 +0000
set GCCGO=gccgo
set GOAMD64=v1
set AR=ar
set CC=gcc
set CXX=g++
set CGO_ENABLED=1
set GOMOD=D:\Projects\goland\it\go.mod
set GOWORK=
set CGO_CFLAGS=-O2 -g
set CGO_CPPFLAGS=
set CGO_CXXFLAGS=-O2 -g
set CGO_FFLAGS=-O2 -g
set CGO_LDFLAGS=-O2 -g
set PKG_CONFIG=pkg-config
set GOGCCFLAGS=-m64 -mthreads -Wl,--no-gc-sections -fmessage-length=0 -ffile-prefix-map=C:\Users\nigel\AppData\Local\Temp\go-build11578654783032352739=/tmp/go-build -gno-record-gcc-switches

What did you do?

GOEXPERIMENT=rangefunc gotip test -race ./race_test.go

the race_test.go file content:

package issue

import (
	"iter"
	"testing"
)

func Range(stop int) iter.Seq2[int, int] {
	return func(yield func(int, int) bool) {
		for i := 0; i < stop; i++ {
			if !yield(i, i) {
				return
			}
		}
	}
}

func TestPull(t *testing.T) {
	r := Range(3)
	next, _ := iter.Pull2(r)
	i, v, ok := next()
	if i != 0 || v != 0 || !ok {
		t.Fail()
	}
}

What did you expect to see?

The test run successfully.

What did you see instead?

A data race error:

==================
WARNING: DATA RACE
Read at 0x00c00009c248 by goroutine 7:
  iter.Pull2[go.shape.int,go.shape.int].func2()
      C:/Users/nigel/sdk/gotip/src/iter/iter.go:151 +0x9b
  command-line-arguments.TestPull()
      D:/Projects/goland/it/issue/race_test.go:21 +0x77
  testing.tRunner()
      C:/Users/nigel/sdk/gotip/src/testing/testing.go:1689 +0x21e
  testing.(*T).Run.gowrap1()
      C:/Users/nigel/sdk/gotip/src/testing/testing.go:1742 +0x44

Previous write at 0x00c00009c248 by goroutine 8:
  iter.Pull2[go.shape.int,go.shape.int].func1.1()
      C:/Users/nigel/sdk/gotip/src/iter/iter.go:135 +0x7d
  command-line-arguments.TestPull.Range.func1()
      D:/Projects/goland/it/issue/race_test.go:11 +0x4d
  iter.Pull2[go.shape.int,go.shape.int].func1()
      C:/Users/nigel/sdk/gotip/src/iter/iter.go:139 +0x207

Goroutine 7 (running) created at:
  testing.(*T).Run()
      C:/Users/nigel/sdk/gotip/src/testing/testing.go:1742 +0x825
  testing.runTests.func1()
      C:/Users/nigel/sdk/gotip/src/testing/testing.go:2161 +0x85
  testing.tRunner()
      C:/Users/nigel/sdk/gotip/src/testing/testing.go:1689 +0x21e
  testing.runTests()
      C:/Users/nigel/sdk/gotip/src/testing/testing.go:2159 +0x8be
  testing.(*M).Run()
      C:/Users/nigel/sdk/gotip/src/testing/testing.go:2027 +0xf17
  main.main()
      _testmain.go:47 +0x2bd

Goroutine 8 (running) created at:
  iter.Pull2[go.shape.int,go.shape.int]()
      C:/Users/nigel/sdk/gotip/src/iter/iter.go:130 +0x264
  command-line-arguments.TestPull()
      D:/Projects/goland/it/issue/race_test.go:20 +0x6f
  testing.tRunner()
      C:/Users/nigel/sdk/gotip/src/testing/testing.go:1689 +0x21e
  testing.(*T).Run.gowrap1()
      C:/Users/nigel/sdk/gotip/src/testing/testing.go:1742 +0x44
==================
==================
WARNING: DATA RACE
Read at 0x00c00009c258 by goroutine 7:
  iter.Pull2[go.shape.int,go.shape.int].func2()
      C:/Users/nigel/sdk/gotip/src/iter/iter.go:151 +0xb2
  command-line-arguments.TestPull()
      D:/Projects/goland/it/issue/race_test.go:21 +0x77
  testing.tRunner()
      C:/Users/nigel/sdk/gotip/src/testing/testing.go:1689 +0x21e
  testing.(*T).Run.gowrap1()
      C:/Users/nigel/sdk/gotip/src/testing/testing.go:1742 +0x44

Previous write at 0x00c00009c258 by goroutine 8:
  iter.Pull2[go.shape.int,go.shape.int].func1.1()
      C:/Users/nigel/sdk/gotip/src/iter/iter.go:135 +0x94
  command-line-arguments.TestPull.Range.func1()
      D:/Projects/goland/it/issue/race_test.go:11 +0x4d
  iter.Pull2[go.shape.int,go.shape.int].func1()
      C:/Users/nigel/sdk/gotip/src/iter/iter.go:139 +0x207

Goroutine 7 (running) created at:
  testing.(*T).Run()
      C:/Users/nigel/sdk/gotip/src/testing/testing.go:1742 +0x825
  testing.runTests.func1()
      C:/Users/nigel/sdk/gotip/src/testing/testing.go:2161 +0x85
  testing.tRunner()
      C:/Users/nigel/sdk/gotip/src/testing/testing.go:1689 +0x21e
  testing.runTests()
      C:/Users/nigel/sdk/gotip/src/testing/testing.go:2159 +0x8be
  testing.(*M).Run()
      C:/Users/nigel/sdk/gotip/src/testing/testing.go:2027 +0xf17
  main.main()
      _testmain.go:47 +0x2bd

Goroutine 8 (running) created at:
  iter.Pull2[go.shape.int,go.shape.int]()
      C:/Users/nigel/sdk/gotip/src/iter/iter.go:130 +0x264
  command-line-arguments.TestPull()
      D:/Projects/goland/it/issue/race_test.go:20 +0x6f
  testing.tRunner()
      C:/Users/nigel/sdk/gotip/src/testing/testing.go:1689 +0x21e
  testing.(*T).Run.gowrap1()
      C:/Users/nigel/sdk/gotip/src/testing/testing.go:1742 +0x44
==================
==================
WARNING: DATA RACE
Read at 0x00c00009c26f by goroutine 7:
  iter.Pull2[go.shape.int,go.shape.int].func2()
      C:/Users/nigel/sdk/gotip/src/iter/iter.go:151 +0xc9
  command-line-arguments.TestPull()
      D:/Projects/goland/it/issue/race_test.go:21 +0x77
  testing.tRunner()
      C:/Users/nigel/sdk/gotip/src/testing/testing.go:1689 +0x21e
  testing.(*T).Run.gowrap1()
      C:/Users/nigel/sdk/gotip/src/testing/testing.go:1742 +0x44

Previous write at 0x00c00009c26f by goroutine 8:
  iter.Pull2[go.shape.int,go.shape.int].func1.1()
      C:/Users/nigel/sdk/gotip/src/iter/iter.go:135 +0xab
  command-line-arguments.TestPull.Range.func1()
      D:/Projects/goland/it/issue/race_test.go:11 +0x4d
  iter.Pull2[go.shape.int,go.shape.int].func1()
      C:/Users/nigel/sdk/gotip/src/iter/iter.go:139 +0x207

Goroutine 7 (running) created at:
  testing.(*T).Run()
      C:/Users/nigel/sdk/gotip/src/testing/testing.go:1742 +0x825
  testing.runTests.func1()
      C:/Users/nigel/sdk/gotip/src/testing/testing.go:2161 +0x85
  testing.tRunner()
      C:/Users/nigel/sdk/gotip/src/testing/testing.go:1689 +0x21e
  testing.runTests()
      C:/Users/nigel/sdk/gotip/src/testing/testing.go:2159 +0x8be
  testing.(*M).Run()
      C:/Users/nigel/sdk/gotip/src/testing/testing.go:2027 +0xf17
  main.main()
      _testmain.go:47 +0x2bd

Goroutine 8 (running) created at:
  iter.Pull2[go.shape.int,go.shape.int]()
      C:/Users/nigel/sdk/gotip/src/iter/iter.go:130 +0x264
  command-line-arguments.TestPull()
      D:/Projects/goland/it/issue/race_test.go:20 +0x6f
  testing.tRunner()
      C:/Users/nigel/sdk/gotip/src/testing/testing.go:1689 +0x21e
  testing.(*T).Run.gowrap1()
      C:/Users/nigel/sdk/gotip/src/testing/testing.go:1742 +0x44
==================
--- FAIL: TestPull (0.00s)
    testing.go:1398: race detected during execution of test
FAIL
FAIL    command-line-arguments  0.633s
FAIL
@mauri870 mauri870 added the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label Dec 11, 2023
@mauri870
Copy link
Member

This can be reproduced just by running the iter tests with -race

GOEXPERIMENT=rangefunc gotip test -race iter

@mauri870 mauri870 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 Dec 11, 2023
@mauri870
Copy link
Member

I see some unresolved comments in the original CL that were left unresolved, I wonder if this is related

https://go-review.googlesource.com/c/go/+/543319

@mauri870 mauri870 mentioned this issue Dec 11, 2023
Closed
@mauri870 mauri870 added this to the Go1.22 milestone Dec 11, 2023
@mauri870
Copy link
Member

@golang/release @rsc Since the iter package will be released with 1.22 I think this fix should be included.

@mauri870 mauri870 changed the title iter: data race in iter.Pull2 iter: incorrect race instrumentation is causing false positives in iter.Pull2 Dec 11, 2023
@mauri870
Copy link
Member

Leaving as release blocker for now.

@dmitshur dmitshur moved this to Todo in Release Blockers Dec 11, 2023
@gopherbot
Copy link
Contributor

Change https://go.dev/cl/548895 mentions this issue: iter: fix race instrumentation for Pull2

@prattmic
Copy link
Member

This only impacts GOEXPERIMENT=rangefunc, so I am tentatively marking this as okay-after-rc1.

@prattmic prattmic added the okay-after-rc1 Used by release team to mark a release-blocker issue as okay to resolve either before or after rc1 label Dec 11, 2023
@mauri870 mauri870 self-assigned this Dec 11, 2023
@github-project-automation github-project-automation bot moved this from Todo to Done in Release Blockers Dec 13, 2023
ezz-no pushed a commit to ezz-no/go-ezzno that referenced this issue Feb 18, 2024
@mauri870 @ezz-no
iter: fix race instrumentation for Pull2
da32c88 
Pull2 tests are failing with -race, giving false-positive race conditions
due to bad race instrumentation.

No tests for this as it should be caught by the race builders. The only
reason it was not caught is because it is behind GOEXPERIMENT=rangefunc.

Fixes golang#64651

Change-Id: I20554da930b0e19594e0e267f01a1e7a9cbc577a
GitHub-Last-Rev: 7c1f192
GitHub-Pull-Request: golang#64653
Reviewed-on: https://go-review.googlesource.com/c/go/+/548895
LUCI-TryBot-Result: Go LUCI <[email protected]>
Reviewed-by: Cherry Mui <[email protected]>
Reviewed-by: Michael Knyszek <[email protected]>
@gabyhelp gabyhelp mentioned this issue Sep 12, 2024
Closed
@golang golang locked and limited conversation to collaborators Dec 12, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees

@mauri870 mauri870

Labels
FrozenDueToAge NeedsFix The path to resolution is known, but the work has not been done. okay-after-rc1 Used by release team to mark a release-blocker issue as okay to resolve either before or after rc1 RaceDetector release-blocker
Projects
Release Blockers
Status: Done
Milestone
Go1.22
Development

Successfully merging a pull request may close this issue.

iter: fix race instrumentation for Pull2 mauri870/go
4 participants
@prattmic @gopherbot @mauri870 @j178