proposal: x/sync/errgroup: Additional goroutines should not be started after the context is cancelled

[nbgraham]

Once a goroutine is ready to run (no longer blocked), errgroup should check if the context has already been cancelled (e.g. the errgroup was created with WithContext and a previous step returned an error). If it has, the next step should not be run.

I guess we would have to check g.cancel != nil to check if the errgroup was created with WithContext to preserve the existing behavior: if not using WithContext, all goroutines should be completed regardless of errors being returned.

(*Group) Go

Add this

if g.cancel != nil  && g.err != nil {
  return
}

here
https://github.com/golang/sync/blob/93782cc822b6b554cb7df40332fd010f0473cbc8/errgroup/errgroup.go#L70

This change might be problematic, because it would mean that calling Go does not guarantee that the function will run. Might have to return a bool or error from the Go func.

(*Group) TryGo

Add this

if g.cancel != nil  && g.err != nil {
  return false
}

here
https://github.com/golang/sync/blob/93782cc822b6b554cb7df40332fd010f0473cbc8/errgroup/errgroup.go#L99

Workaround

I can achieve this behavior today like this, but it feels like something that should be included.

	g, ctx := errgroup.WithContext(context.Background()) // NEW
	g.SetLimit(1) // NEW
	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
		g.Go(func() error {
			// NEW
			if err := ctx.Err(); err != nil {
				return err
			}

			// Fetch the URL.
			resp, err := http.Get(url)
			if err == nil {
				resp.Body.Close()
			}
			return err
		})
	}
	// Wait for all HTTP fetches to complete.
	if err := g.Wait(); err == nil {
		fmt.Println("Successfully fetched all URLs.")
	}

[seankhliao]

Duplicate of #54045