fix cancellation error not being detected and erroneously cached

Signed-off-by: Tonis Tiigi <[email protected]> (cherry picked from commit 6644f16) Signed-off-by: Tonis Tiigi <[email protected]>
moby · Aug 22, 2022 · 36e7f50 · 36e7f50
1 parent cd6fdf0
commit 36e7f50
Show file tree

Hide file tree

Showing 5 changed files with 22 additions and 10 deletions.
diff --git a/frontend/gateway/gateway.go b/frontend/gateway/gateway.go
@@ -278,7 +278,7 @@ func (gf *gatewayFrontend) Solve(ctx context.Context, llbBridge frontend.Fronten
 	err = w.Executor().Run(ctx, "", mountWithSession(rootFS, session.NewGroup(sid)), mnts, executor.ProcessInfo{Meta: meta, Stdin: lbf.Stdin, Stdout: lbf.Stdout, Stderr: os.Stderr}, nil)
 
 	if err != nil {
-		if errdefs.IsCanceled(err) && lbf.isErrServerClosed {
+		if errdefs.IsCanceled(ctx, err) && lbf.isErrServerClosed {
 			err = errors.Errorf("frontend grpc server closed unexpectedly")
 		}
 		// An existing error (set via Return rpc) takes

diff --git a/solver/errdefs/context.go b/solver/errdefs/context.go
@@ -3,11 +3,25 @@ package errdefs
 import (
 	"context"
 	"errors"
+	"strings"
 
 	"github.com/moby/buildkit/util/grpcerrors"
 	"google.golang.org/grpc/codes"
 )
 
-func IsCanceled(err error) bool {
-	return errors.Is(err, context.Canceled) || grpcerrors.Code(err) == codes.Canceled
+func IsCanceled(ctx context.Context, err error) bool {
+	if errors.Is(err, context.Canceled) || grpcerrors.Code(err) == codes.Canceled {
+		return true
+	}
+	// grpc does not set cancel correctly when stream gets cancelled and then Recv is called
+	if err != nil && ctx.Err() == context.Canceled {
+		// when this error comes from containerd it is not typed at all, just concatenated string
+		if strings.Contains(err.Error(), "EOF") {
+			return true
+		}
+		if strings.Contains(err.Error(), context.Canceled.Error()) {
+			return true
+		}
+	}
+	return false
 }
diff --git a/solver/jobs.go b/solver/jobs.go
@@ -3,7 +3,6 @@ package solver
 import (
 	"context"
 	"fmt"
-	"strings"
 	"sync"
 	"time"
 
@@ -705,7 +704,7 @@ func (s *sharedOp) CalcSlowCache(ctx context.Context, index Index, p PreprocessF
 		if err != nil {
 			select {
 			case <-ctx.Done():
-				if strings.Contains(err.Error(), context.Canceled.Error()) {
+				if errdefs.IsCanceled(ctx, err) {
 					complete = false
 					releaseError(err)
 					err = errors.Wrap(ctx.Err(), err.Error())
@@ -771,7 +770,7 @@ func (s *sharedOp) CacheMap(ctx context.Context, index int) (resp *cacheMapResp,
 		if err != nil {
 			select {
 			case <-ctx.Done():
-				if strings.Contains(err.Error(), context.Canceled.Error()) {
+				if errdefs.IsCanceled(ctx, err) {
 					complete = false
 					releaseError(err)
 					err = errors.Wrap(ctx.Err(), err.Error())
@@ -841,7 +840,7 @@ func (s *sharedOp) Exec(ctx context.Context, inputs []Result) (outputs []Result,
 		if err != nil {
 			select {
 			case <-ctx.Done():
-				if strings.Contains(err.Error(), context.Canceled.Error()) {
+				if errdefs.IsCanceled(ctx, err) {
 					complete = false
 					releaseError(err)
 					err = errors.Wrap(ctx.Err(), err.Error())

diff --git a/solver/llbsolver/bridge.go b/solver/llbsolver/bridge.go
@@ -3,7 +3,6 @@ package llbsolver
 import (
 	"context"
 	"fmt"
-	"strings"
 	"sync"
 	"time"
 
@@ -290,7 +289,7 @@ func (rp *resultProxy) Result(ctx context.Context) (res solver.CachedResult, err
 		if err != nil {
 			select {
 			case <-ctx.Done():
-				if strings.Contains(err.Error(), context.Canceled.Error()) {
+				if errdefs.IsCanceled(ctx, err) {
 					return v, err
 				}
 			default:

diff --git a/source/containerimage/pull.go b/source/containerimage/pull.go
@@ -182,7 +182,7 @@ func (p *puller) CacheKey(ctx context.Context, g session.Group, index int) (cach
 			return nil, p.cacheKeyErr
 		}
 		defer func() {
-			if !errdefs.IsCanceled(err) {
+			if !errdefs.IsCanceled(ctx, err) {
 				p.cacheKeyErr = err
 			}
 		}()