x/sync/errgroup: Group: document that Go may not be concurrent with Wait unless semaphore > 0

[haaawk]

(Edit: skip down to #70284 (comment); this is now a doc change request. --@adonovan)

Go version

go version go1.23.1 darwin/amd64

Output of go env in your module/workspace:

GO111MODULE=''
GOARCH='amd64'
GOBIN=''
GOCACHE='/Users/haaawk/Library/Caches/go-build'
GOENV='/Users/haaawk/Library/Application Support/go/env'
GOEXE=''
GOEXPERIMENT=''
GOFLAGS=''
GOHOSTARCH='amd64'
GOHOSTOS='darwin'
GOINSECURE=''
GOMODCACHE='/Users/haaawk/go/pkg/mod'
GONOPROXY=''
GONOSUMDB=''
GOOS='darwin'
GOPATH='/Users/haaawk/go'
GOPRIVATE=''
GOPROXY='https://proxy.golang.org,direct'
GOROOT='/usr/local/go'
GOSUMDB='sum.golang.org'
GOTMPDIR=''
GOTOOLCHAIN='auto'
GOTOOLDIR='/usr/local/go/pkg/tool/darwin_amd64'
GOVCS=''
GOVERSION='go1.23.1'
GODEBUG=''
GOTELEMETRY='local'
GOTELEMETRYDIR='/Users/haaawk/Library/Application Support/go/telemetry'
GCCGO='gccgo'
GOAMD64='v1'
AR='ar'
CC='clang'
CXX='clang++'
CGO_ENABLED='1'
GOMOD='/dev/null'
GOWORK=''
CGO_CFLAGS='-O2 -g'
CGO_CPPFLAGS=''
CGO_CXXFLAGS='-O2 -g'
CGO_FFLAGS='-O2 -g'
CGO_LDFLAGS='-O2 -g'
PKG_CONFIG='pkg-config'
GOGCCFLAGS='-fPIC -arch x86_64 -m64 -pthread -fno-caret-diagnostics -Qunused-arguments -fmessage-length=0 -ffile-prefix-map=/var/folders/_y/nj5hcsh93l12tmsszxgx7ntr0000gn/T/go-build2396141927=/tmp/go-build -gno-record-gcc-switches -fno-common'

What did you do?

When following code is run:

package main

import (
	"runtime"

	"golang.org/x/sync/errgroup"
)

func main() {
	runtime.GOMAXPROCS(1)
	g := &errgroup.Group{}
	g.SetLimit(1)
	ch := make(chan struct{})
	wait := make(chan struct{}, 2)
	g.Go(func() error {
		<-ch
		wait <- struct{}{}
		return nil
	})
	go g.Go(func() error {
		println("I'm not blocked")
		wait <- struct{}{}
		return nil
	})
	println("Ok let's play")
	close(ch)
	g.Wait()
	println("It's over already?")
	<-wait
	<-wait
}

https://go.dev/play/p/xTIsT1iouTd

What did you see happen?

The program printed:

Ok let's play
It's over already?
I'm not blocked

What did you expect to see?

The program printing:

Ok let's play
I'm not blocked
It's over already?

[gabyhelp]

Related Issues

• golang.org/x/sync/errgroup: strange data race #65045 (closed)
• Importing net/http prevents goroutine deadlock error in particular case #40489 (closed)
• x/sync/errgroup: clarify that Go calls the function regardless of prior errors #54045 (closed)
• testing: inconsistent behaviors between running tests directly and after compiling the code first #59879 (closed)
• affected/package: os/exec, context not work. #53700 (closed)
• cmd/go: test gets hung #27720 (closed)
• sync: wait group produces data race when waiting before adding in separate goroutine #56728 (closed)
• affected/package: sync #50544 (closed)
• need help: different call using the same channel. #35094 (closed)

Related Discussions

• Potential Issue in errgroup

_{(Emoji vote if this was helpful or unhelpful; more detailed feedback welcome in this discussion.)}

[haaawk]

I send a patch here https://go-review.googlesource.com/c/sync/+/627075

[gopherbot]

Change https://go.dev/cl/627075 mentions this issue: x/sync/errgroup: ensure all goroutines finish before Wait returns

[jrick]

go g.Go(

I don't think this is what you meant to write.

[haaawk]

go g.Go(

I don't think this is what you meant to write.

It is exactly what I meant to write. Otherwise the g.Go would block.

[jrick]

Your program contains a data race, because it's not valid to increment a waitgroup with 0 count while waiting on it. Your submitted patch doesn't change this.

[haaawk]

Your program contains a data race, because it's not valid to increment a waitgroup with 0 count while waiting on it. Your submitted patch doesn't change this.

The waitgroup has count of 1 when the go g.Go is called

[jrick]

No. You would need additional synchronization to guarantee this.

package main

import (
        "runtime"

        "golang.org/x/sync/errgroup"
)

func main() {
        runtime.GOMAXPROCS(1)
        g := &errgroup.Group{}
        g.SetLimit(1)
        ch := make(chan struct{})
        wait := make(chan struct{}, 2)
        insideGo := make(chan struct{})
        g.Go(func() error {
                <-ch
                wait <- struct{}{}
                return nil
        })
        go g.Go(func() error {
                close(insideGo)
                println("I'm not blocked")
                wait <- struct{}{}
                return nil
        })
        println("Ok let's play")
        close(ch)
        <-insideGo
        g.Wait()
        println("It's over already?")
        <-wait
        <-wait
}

[cherrymui]

@jrick is right. go g.Go(...) is not what you want. g.Go is intentionally blocking to limit the number of active goroutines. You've called g.SetLimit(1), so that limits it to 1 active goroutine a time. If you don't want that limit, you can remove that line, or set a higher limit.

In your case, there is no synchronization between g.Wait and the g.Go call in the new goroutine created by go g.Go(...). If is possible that g.Wait has finished while the new goroutine created by go g.Go(...) hasn't run. So this works as intended. Your CL doesn't seem to change it either: g.Wait can still finish even before the new goroutine reaches g.Go.

In general, if you have questions about how to use the API, please see https://go.dev/wiki/Questions . Thanks.

[adonovan]

Perhaps I misunderstand, but I think the Group is working as intended. SetLimit may prevent the Group from accepting new work, and the client must deal with that. It definitely cannot call Go asynchronously as there would be no guarantee that it happened before the later call to Wait.

Perhaps the documentation could be improved, but I don't think there's a bug.

[adonovan]

Ah, @cherrymui beat me to it!

[haaawk]

This was just my failed attempt to have a simple reproducer to the issue that's still there.
In the code below the order is always right and the program still prints the same result. The problem is that g.Go can wake up from sem in a group that was already waited on.
Let me thing of less artificial example and come back. It will need to be more complex unfortunately.

package main

import (
	"runtime"
	"time"

	"golang.org/x/sync/errgroup"
)

func main() {
	runtime.GOMAXPROCS(1)
	g := &errgroup.Group{}
	g.SetLimit(1)
	ch := make(chan struct{})
	wait := make(chan struct{}, 2)
	g.Go(func() error {
		<-ch
		wait <- struct{}{}
		return nil
	})
	go g.Go(func() error {
		println("I'm not blocked")
		wait <- struct{}{}
		return nil
	})
	println("Ok let's play")
	time.Sleep(1 * time.Second)
	close(ch)
	g.Wait()
	println("It's over already?")
	<-wait
	<-wait
}

[adonovan]

In the code below the order is always right and the program still prints the same result.

This program has a race: the second call to Go races with Wait. Consider: the goroutine created by the first Go function could complete (along with println, Sleep, close) before the goroutine created by the go statement even begins to execute the second call to g.Go.