sync: add WaitGroup.Go

[dolmen]

TL;DR

Add a func (*WaitGroup) Go(task func()) method to launch a task in a goroutine tracked with a sync.WaitGroup.

Combined with the loopvar change (#60078), writing parallel code would be much less error prone.

Rationale

A very common use case for sync.WaitGroup is to track the termination of tasks launched in goroutines.

Here is the classic example:

	var wg sync.WaitGroup
	for i := 1; i <= 5; i++ {
		i := i
		wg.Add(1)
		go func() {
			defer wg.Done()
			work(i)
		}()
	}
	wg.Wait()

I propose to add a func (*WaitGroup) Go(func()) method that would wrap:

• wg.Add(1): the 1 looks like a magic value
• launching the goroutine: the go keyword and the () after the func body are magic for Go beginners
• defer wg.Done(): the defer keyword and the appearance of Done before the call to the worker are magic

A simple implementation:

func (wg *WaitGroup) Go(task func()) {
	wg.Add(1)
	go func() {
		defer wg.Done()
		task()
	}()
}

The example above would be much reduced and many footguns avoided (the last remaining footgun is being addressed by #60078):

	var wg WaitGroup
	for i := 1; i <= 5; i++ {
		i := i  // avoid loopvar footgun for go < 1.22
		wg.Go(func() {
			work(i)
		})
	}
	wg.Wait()

The full modified example, including an extended implementation of sync.WaitGroup, is available on the Go playground.

(to handle the case of a task with arguments, I would recommend rewriting the work to use the builder pattern: https://go.dev/play/p/g1Um_GhQOyc)

[dsnet]

This is a repeat of #18022, which was re-oriented to become a vet check, but I'm still highly in support of a WaitGroup.Go method. The vet check has never been implemented so it's still easy for users to write the racy version where they increment the WaitGroup within the goroutine rather than before it.

The tailscale codebase has an extension to sync.WaitGroup that adds the Go method. In our usages of it, I have found the Go method to be cleaner.

As an additional data point, the errgroup.Group type also has a Go method.

[seankhliao]

There's also #57534 to just add errgroup in std

[dolmen]

@dsnet Thanks for the link. GitHub search just doesn't work and didn't help me to (re)discover prior proposals.

[dolmen]

See also golang.org/x/sync/errgroup.Group.Go(func() error)

[earthboundkid]

ISTM that sync might get a v2 to add typed sync.Map. It could be worth waiting for that to then adopt the errgroup methods on a new type.

[timothy-king]

Maybe Go could be added to a different type in sync other than sync.WaitGroup? The real benefit of adding Go to sync.WaitGroup is to mix the old paradigm of Add and Done with the new paradigm of Go on the same object. Mixed usage sounds more confusing than keeping each type narrower and clearer. (My suggestion is essentially the same as WaitGroupX in the playground example.)

[earthboundkid]

A WaitGroup with a Go method is not very different from https://pkg.go.dev/cmd/go/internal/par#Queue. I would kind of rather see that moved out of internal and into sync/par. It's a much higher level library than the sync primitives.

[ConradIrwin]

I do think this would be nice to have on wait group.

A very relevant piece of writing is this: https://vorpus.org/blog/notes-on-structured-concurrency-or-go-statement-considered-harmful/. He argues that go routines are too flexible, and a more restrictive primitive is usually what you want.

At a previous employer we used https://github.com/ConradIrwin/parallel instead of either waitgroup or go statements. This let us do nice things like ensure that integration tests wait for all spawned go-routines before completing.

I'm not sure that waitgroup should gain Do too; but I do like the idea of having a shared implementation of structured concurrency in go. (On the flip side, the advantage of structured concurrency is that a correct implementation is transparent to callers).

[earthboundkid]

I was also inspired by the structured concurrency post when I wrote my helper library https://github.com/carlmjohnson/flowmatic. It's a little tricky because the standard library needs to have low level primitives so that people can build higher level abstractions like Map and whatnot, but it would be good to have a standard set of high level abstractions too.

[Merovius]

I've also wanted (*WaitGroup).Go a few times. To me, errgroup serves a slightly different purpose with slightly different semantics and I still use WaitGroups all the time. Having a convenient Go method would simplify things and I don't really see a whole lot of downsides, to be honest.

[dsnet]

I don't really see a whole lot of downsides, to be honest.

One possible argument against it is that use of it requires the function to close over variable that may be mutating over the lifetime of the asynchronously running goroutine. As it stands today, people can write the more safe:

wg.Add(1)
go func(a int, b string) {
    defer wg.Done()
    ...
}(a, b)

where a and b are passed in as explicit arguments, and avoid racy mutations.

However, my personal experience is that I've encountered more bugs due to calling wg.Add(1) in the wrong place:

go func(a int, b string) {
    wg.Add(1)
    defer wg.Done()
    ...
}()