From 2bb8ce2f57ebd058c5954c4f9f3a5bd1669b768a Mon Sep 17 00:00:00 2001 From: Justin Chadwell Date: Tue, 10 Jan 2023 11:02:26 +0000 Subject: [PATCH] build: create error group per opt Using the syncronization primitive, we can avoid needing to create a separate wait group. This allows us to sidestep the issue where the wait group could be completed, but the build invocation functions had not terminated - if one of the functions was to terminate with an error, then it was possible to encounter a race condition, where the result handling code would begin executing, despite an error. The refactor to use a separate error group which more elegantly handles the concept of function returns and errors, ensures that we can't encounter this issue. Signed-off-by: Justin Chadwell (cherry picked from commit 8b7aa1a168c303521cfb6613c52480cd71f1ef20) --- build/build.go | 15 ++++++--------- 1 file changed, 6 insertions(+), 9 deletions(-) diff --git a/build/build.go b/build/build.go index 84f3eb2c..876245e0 100644 --- a/build/build.go +++ b/build/build.go @@ -952,10 +952,10 @@ func BuildWithResultHandler(ctx context.Context, nodes []builder.Node, opt map[s if multiTarget { span, ctx = tracing.StartSpan(ctx, k) } + baseCtx := ctx res := make([]*client.SolveResponse, len(dps)) - wg := &sync.WaitGroup{} - wg.Add(len(dps)) + eg2, ctx := errgroup.WithContext(ctx) var pushNames string var insecurePush bool @@ -993,9 +993,8 @@ func BuildWithResultHandler(ctx context.Context, nodes []builder.Node, opt map[s pw := progress.WithPrefix(w, k, multiTarget) c := clients[dp.driverIndex] - eg.Go(func() error { + eg2.Go(func() error { pw = progress.ResetTime(pw) - defer wg.Done() if err := waitContextDeps(ctx, dp.driverIndex, results, &so); err != nil { return err @@ -1128,17 +1127,15 @@ func BuildWithResultHandler(ctx context.Context, nodes []builder.Node, opt map[s } eg.Go(func() (err error) { + ctx := baseCtx defer func() { if span != nil { tracing.FinishWithError(span, err) } }() pw := progress.WithPrefix(w, "default", false) - wg.Wait() - select { - case <-ctx.Done(): - return ctx.Err() - default: + if err := eg2.Wait(); err != nil { + return err } respMu.Lock()