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

sync: wait group produces data race when waiting before adding in separate goroutine #56728

Closed
alarbada opened this issue Nov 14, 2022 · 1 comment
Closed

sync: wait group produces data race when waiting before adding in separate goroutine #56728

alarbada opened this issue Nov 14, 2022 · 1 comment
Labels
compiler/runtime Issues related to the Go compiler and/or runtime. FrozenDueToAge

Comments

@alarbada
Copy link

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

$ go version
go version go1.19.1 linux/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
GO111MODULE=""
GOARCH="amd64"
GOBIN=""
GOCACHE="/home/guillem/.cache/go-build"
GOENV="/home/guillem/.config/go/env"
GOEXE=""
GOEXPERIMENT=""
GOFLAGS=""
GOHOSTARCH="amd64"
GOHOSTOS="linux"
GOINSECURE=""
GOMODCACHE="/home/guillem/golang/pkg/mod"
GONOPROXY=""
GONOSUMDB=""
GOOS="linux"
GOPATH="/home/guillem/golang"
GOPRIVATE=""
GOPROXY="https://proxy.golang.org,direct"
GOROOT="/usr/local/go"
GOSUMDB="sum.golang.org"
GOTMPDIR=""
GOTOOLDIR="/usr/local/go/pkg/tool/linux_amd64"
GOVCS=""
GOVERSION="go1.19.1"
GCCGO="gccgo"
GOAMD64="v1"
AR="ar"
CC="gcc"
CXX="g++"
CGO_ENABLED="1"
CGO_CFLAGS="-g -O2"
CGO_CPPFLAGS=""
CGO_CXXFLAGS="-g -O2"
CGO_FFLAGS="-g -O2"
CGO_LDFLAGS="-g -O2"
PKG_CONFIG="pkg-config"
GOGCCFLAGS="-fPIC -m64 -pthread -Wl,--no-gc-sections -fmessage-length=0 -fdebug-prefix-map=/tmp/go-build3293673385=/tmp/go-build -gno-record-gcc-switches"

What did you do?

Run this test with the -race flag:

func TestThing(t *testing.T) {
	var wg sync.WaitGroup
	someChannel := make(chan int)

        // waiting before adding anything, racy, but don't know why.
	go func() {
		wg.Wait()
		close(someChannel)
	}()

	for i := 0; i < 100; i++ {
		wg.Add(1)
		i := i
		go func() {
			defer wg.Done()
			someChannel <- i
		}()
	}

	var sum int
	for i := range someChannel {
		sum += i
	}
}

This racy behaviour is not documented at the sync.Waitgroup docs, so maybe it's a bug? Didn't expect a data race. Maybe a panic due to a send to a closed channel, but not a data race.

This fixes it:

func TestThing(t *testing.T) {
	var wg sync.WaitGroup
	someChannel := make(chan int)

	for i := 0; i < 100; i++ {
		wg.Add(1)
		i := i
		go func() {
			defer wg.Done()
			someChannel <- i
		}()
	}

	// waiting after adding, works as expected
	go func() {
		wg.Wait()
		close(someChannel)
	}()


	var sum int
	for i := range someChannel {
		sum += i
	}
}
@gopherbot gopherbot added the compiler/runtime Issues related to the Go compiler and/or runtime. label Nov 14, 2022
@randall77
Copy link
Contributor

From https://pkg.go.dev/sync#WaitGroup.Add

Note that calls with a positive delta that occur when the counter is zero must happen before a Wait. Calls with a negative delta, or calls with a positive delta that start when the counter is greater than zero, may happen at any time. Typically this means the calls to Add should execute before the statement creating the goroutine or other event to be waited for.

I think your example is violating that rule.

@seankhliao seankhliao closed this as not planned Won't fix, can't repro, duplicate, stale Nov 14, 2022
@golang golang locked and limited conversation to collaborators Nov 14, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
compiler/runtime Issues related to the Go compiler and/or runtime. FrozenDueToAge
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
4 participants
@randall77 @gopherbot @seankhliao @alarbada