Go: proposal: sync: add WaitGroup.Go method

[oguzyildiz1991]

When I use errgroup.Group, I noticed I need much less code compared to sync.WaitGroup in certain situations. While keeping the existing methods of WaitGroup, can we add a Go(func()) function to make it more seamless? (And while at it, we could even make the Go method return the WaitGroup itself for chained calls? I wouldn't insist on this but it would be nice instead of it returning nothing)
It will be like:

wg := sync.WaitGroup{}
wg.Add(1)
go func() {
    defer wg.Done()
    doWork()  
}()

vs

wg := sync.WaitGroup{}.Go(func(){
    doWork()
})

or potentially:

wg := sync.WaitGroup{}.Go(doWork)

I feel like it's not a crazy far-fetched idea since errgroup.Group already has this method.

~And this would be a nice addition for semaphore.Weighted as well, what do you think?~

[davecheney]

Could you explain more about the use cases here. From the example you gave, you’re running a function in a goroutine and waiting for it to finish — that’s the same as calling the function directly.

[ianlancetaylor]

CC @bcmills

[oguzyildiz1991]

@davecheney of course that's not the whole usage :) I just didn't want to write more code than necessary to make it easier to read. I think it could apply any place you use WaitGroup where you're waiting other go routines to finish their computation.

[davecheney]

If you could give some examples that were closer to your intended usage that might help make your proposal more compelling.

[urandom]

@davecheney
Perhaps the proposal is about aligning the WaitGroup API more towards ErrGroup's

wg := sync.WaitGroup{}
var urls = []string{
    "http://www.golang.org/",
    "http://www.google.com/",
    "http://www.somestupidname.com/",
}
for _, url := range urls {
    // Launch a goroutine to fetch the URL.
    url := url // https://golang.org/doc/faq#closures_and_goroutines
    wg.Go(func() {
        // Fetch the URL.
        resp, err := http.Get(url)
        if err == nil {
            resp.Body.Close()
        }
    })
}
// Wait for all HTTP fetches to complete.
if err := wg.Wait(); err == nil {
    fmt.Println("Successfully fetched all URLs.")
}

[bcmills]

And this would be a nice addition for semaphore.Weighted as well, what do you think?

I would rather add a semaphore to errgroup (#27837) than add goroutine-management to semaphore.Weighted.

[bcmills]

I feel like it's not a crazy far-fetched idea since errgroup.Group already has this method.

I agree that this would be a reasonable addition to sync.WaitGroup, but note that the analogous API in errgroup.Group is much harder for callers to get right due to the error return value. (Using a context.WithContext and sync.WaitGroup directly, it's much too easy to accidentally cancel the Context too early and store a secondary error that occurs as a result, instead of the intended primary-cause error.)

Since sync.WaitGroup doesn't deal with errors or return values, there is less (but still non-trivial) risk of mismatched-Add bugs at the call site.

[bcmills]

See previously #23538 and #18022.

[oguzyildiz1991]

@davecheney This is a very common pattern in our codebase, say I want to run 3 things in parallel. It'll be this:

wg := sync.WaitGroup{}
wg.Add(2)
go func() {
    defer wg.Done()
    doThis()  
}()
go func() {
    defer wg.Done()
    doThat()  
}()
doThose()
wg.Wait()

vs this:

wg := sync.WaitGroup{}
wg.Go(doThis)
wg.Go(doThat)
doThose()
wg.Wait()

This is also less bug prone for the cases when people call wrong amount of Add or Done

[networkimprov]

I've used a similar pattern that passes the WaitGroup to ops which may run independently:

wg := sync.WaitGroup{}
wg.Add(1); go doThis(&wg)
wg.Add(1); go doThat(&wg)
doThose(nil)
wg.Wait()

[rsc]

I can't find the previous discussion, although I'm sure there is one.
The general problem with this kind of method is that go, like defer, is carefully designed to give control over evaluating some parts of the call - namely the function being called and the arguments - in the original goroutine and only running the call itself in the new goroutine. Any Go method can't do that - it has to take a func(), with the pre-evaluation left to the caller (if the caller remembers!). That makes it a lot more clunky, and is why we've encouraged not wrapping the go statement and instead simply using it.

As for the chained ("fluent") calls, we've avoided that pattern in the Go standard library so far. It's just not idiomatic Go at this point. If you want to remember the receiver, use a variable.

If the problem is forgetting the wg.Add to match wg.Done, it still seems like a vet check would be best (#18022). I'm not sure we ever did that.

[rsc]

Now I see that the previous discussion _is_ #18022.
It still seems like we should try to figure out a good, accurate, no-false-positive way to check this in vet.

[rsc]

Based on the discussion above, I suggest we decline this issue and continue discussion of the vet check on the previously-filed #18022. Does anyone object to that?

[rsc]

Based on the discussion above, this seems like a likely decline.

[rsc]

No change in consensus, so declined.