Skip to content

Commit

Permalink
replace WithTimeout with WithTimeoutCause
Browse files Browse the repository at this point in the history
Signed-off-by: Tonis Tiigi <tonistiigi@gmail.com>
  • Loading branch information
tonistiigi committed Dec 6, 2023
1 parent 70ab3ca commit 86ad76e
Show file tree
Hide file tree
Showing 24 changed files with 88 additions and 61 deletions.
2 changes: 2 additions & 0 deletions .golangci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,8 @@ linters-settings:
- '^fmt\.Errorf(# use errors\.Errorf instead)?$'
- '^logrus\.(Trace|Debug|Info|Warn|Warning|Error|Fatal)(f|ln)?(# use bklog\.G or bklog\.L instead of logrus directly)?$'
- '^context\.WithCancel(# use context\.WithCancelCause instead)?$'
- '^context\.WithTimeout(# use context\.WithTimeoutCause instead)?$'
- '^context\.WithDeadline(# use context\.WithDeadline instead)?$'
- '^ctx\.Err(# use context\.Cause instead)?$'
importas:
alias:
Expand Down
10 changes: 6 additions & 4 deletions cache/remotecache/azblob/exporter.go
Original file line number Diff line number Diff line change
Expand Up @@ -150,8 +150,9 @@ func (ce *exporter) uploadManifest(ctx context.Context, manifestKey string, read
return errors.Wrap(err, "error creating container client")
}

ctx, cnclFn := context.WithTimeout(ctx, time.Minute*5)
defer cnclFn()
ctx, cnclFn := context.WithCancelCause(ctx)
ctx, _ = context.WithTimeoutCause(ctx, time.Minute*5, errors.WithStack(context.DeadlineExceeded))
defer cnclFn(errors.WithStack(context.Canceled))

_, err = blobClient.Upload(ctx, reader, &azblob.BlockBlobUploadOptions{})
if err != nil {
Expand All @@ -170,8 +171,9 @@ func (ce *exporter) uploadBlobIfNotExists(ctx context.Context, blobKey string, r
return errors.Wrap(err, "error creating container client")
}

uploadCtx, cnclFn := context.WithTimeout(ctx, time.Minute*5)
defer cnclFn()
uploadCtx, cnclFn := context.WithCancelCause(ctx)
uploadCtx, _ = context.WithTimeoutCause(uploadCtx, time.Minute*5, errors.WithStack(context.DeadlineExceeded))
defer cnclFn(errors.WithStack(context.Canceled))

// Only upload if the blob doesn't exist
eTagAny := azblob.ETagAny
Expand Down
15 changes: 9 additions & 6 deletions cache/remotecache/azblob/utils.go
Original file line number Diff line number Diff line change
Expand Up @@ -133,8 +133,9 @@ func createContainerClient(ctx context.Context, config *Config) (*azblob.Contain
}
}

ctx, cnclFn := context.WithTimeout(ctx, time.Second*60)
defer cnclFn()
ctx, cnclFn := context.WithCancelCause(ctx)
ctx, _ = context.WithTimeoutCause(ctx, time.Second*60, errors.WithStack(context.DeadlineExceeded))
defer cnclFn(errors.WithStack(context.Canceled))

containerClient, err := serviceClient.NewContainerClient(config.Container)
if err != nil {
Expand All @@ -148,8 +149,9 @@ func createContainerClient(ctx context.Context, config *Config) (*azblob.Contain

var se *azblob.StorageError
if errors.As(err, &se) && se.ErrorCode == azblob.StorageErrorCodeContainerNotFound {
ctx, cnclFn := context.WithTimeout(ctx, time.Minute*5)
defer cnclFn()
ctx, cnclFn := context.WithCancelCause(ctx)
ctx, _ = context.WithTimeoutCause(ctx, time.Minute*5, errors.WithStack(context.DeadlineExceeded))
defer cnclFn(errors.WithStack(context.Canceled))
_, err := containerClient.Create(ctx, &azblob.ContainerCreateOptions{})
if err != nil {
return nil, errors.Wrapf(err, "failed to create cache container %s", config.Container)
Expand Down Expand Up @@ -177,8 +179,9 @@ func blobExists(ctx context.Context, containerClient *azblob.ContainerClient, bl
return false, errors.Wrap(err, "error creating blob client")
}

ctx, cnclFn := context.WithTimeout(ctx, time.Second*60)
defer cnclFn()
ctx, cnclFn := context.WithCancelCause(ctx)
ctx, _ = context.WithTimeoutCause(ctx, time.Second*60, errors.WithStack(context.DeadlineExceeded))
defer cnclFn(errors.WithStack(context.Canceled))
_, err = blobClient.GetProperties(ctx, &azblob.BlobGetPropertiesOptions{})
if err == nil {
return true, nil
Expand Down
5 changes: 3 additions & 2 deletions cache/remotecache/local/local.go
Original file line number Diff line number Diff line change
Expand Up @@ -105,8 +105,9 @@ func getContentStore(ctx context.Context, sm *session.Manager, g session.Group,
if sessionID == "" {
return nil, errors.New("local cache exporter/importer requires session")
}
timeoutCtx, cancel := context.WithTimeout(context.Background(), 5*time.Second)
defer cancel()
timeoutCtx, cancel := context.WithCancelCause(context.Background())
timeoutCtx, _ = context.WithTimeoutCause(timeoutCtx, 5*time.Second, errors.WithStack(context.DeadlineExceeded))
defer cancel(errors.WithStack(context.Canceled))

caller, err := sm.Get(timeoutCtx, sessionID, false)
if err != nil {
Expand Down
10 changes: 5 additions & 5 deletions client/build_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -933,7 +933,7 @@ func testClientGatewayContainerPID1Tty(t *testing.T, sb integration.Sandbox) {
output := bytes.NewBuffer(nil)

b := func(ctx context.Context, c client.Client) (*client.Result, error) {
ctx, timeout := context.WithTimeout(ctx, 10*time.Second)
ctx, timeout := context.WithTimeoutCause(ctx, 10*time.Second, nil)
defer timeout()

st := llb.Image("busybox:latest")
Expand Down Expand Up @@ -1015,7 +1015,7 @@ func testClientGatewayContainerCancelPID1Tty(t *testing.T, sb integration.Sandbo
output := bytes.NewBuffer(nil)

b := func(ctx context.Context, c client.Client) (*client.Result, error) {
ctx, cancel := context.WithTimeout(ctx, 10*time.Second)
ctx, cancel := context.WithTimeoutCause(ctx, 10*time.Second, nil)
defer cancel()

st := llb.Image("busybox:latest")
Expand Down Expand Up @@ -1141,7 +1141,7 @@ func testClientGatewayContainerExecTty(t *testing.T, sb integration.Sandbox) {
inputR, inputW := io.Pipe()
output := bytes.NewBuffer(nil)
b := func(ctx context.Context, c client.Client) (*client.Result, error) {
ctx, timeout := context.WithTimeout(ctx, 10*time.Second)
ctx, timeout := context.WithTimeoutCause(ctx, 10*time.Second, nil)
defer timeout()
st := llb.Image("busybox:latest")

Expand Down Expand Up @@ -1233,7 +1233,7 @@ func testClientGatewayContainerCancelExecTty(t *testing.T, sb integration.Sandbo
inputR, inputW := io.Pipe()
output := bytes.NewBuffer(nil)
b := func(ctx context.Context, c client.Client) (*client.Result, error) {
ctx, timeout := context.WithTimeout(ctx, 10*time.Second)
ctx, timeout := context.WithTimeoutCause(ctx, 10*time.Second, nil)
defer timeout()
st := llb.Image("busybox:latest")

Expand Down Expand Up @@ -2132,7 +2132,7 @@ func testClientGatewayContainerSignal(t *testing.T, sb integration.Sandbox) {
product := "buildkit_test"

b := func(ctx context.Context, c client.Client) (*client.Result, error) {
ctx, timeout := context.WithTimeout(ctx, 10*time.Second)
ctx, timeout := context.WithTimeoutCause(ctx, 10*time.Second, nil)
defer timeout()

st := llb.Image("busybox:latest")
Expand Down
2 changes: 1 addition & 1 deletion client/client_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -9832,7 +9832,7 @@ func testLLBMountPerformance(t *testing.T, sb integration.Sandbox) {
def, err := st.Marshal(sb.Context())
require.NoError(t, err)

timeoutCtx, cancel := context.WithTimeout(sb.Context(), time.Minute)
timeoutCtx, cancel := context.WithTimeoutCause(sb.Context(), time.Minute, nil)
defer cancel()
_, err = c.Solve(timeoutCtx, def, SolveOpt{}, nil)
require.NoError(t, err)
Expand Down
5 changes: 3 additions & 2 deletions cmd/buildctl/common/common.go
Original file line number Diff line number Diff line change
Expand Up @@ -91,9 +91,10 @@ func ResolveClient(c *cli.Context) (*client.Client, error) {

timeout := time.Duration(c.GlobalInt("timeout"))
if timeout > 0 {
ctx2, cancel := context.WithTimeout(ctx, timeout*time.Second)
ctx2, cancel := context.WithCancelCause(ctx)
ctx2, _ = context.WithTimeoutCause(ctx2, timeout*time.Second, errors.WithStack(context.DeadlineExceeded))
ctx = ctx2
defer cancel()
defer cancel(errors.WithStack(context.Canceled))
}

cl, err := client.New(ctx, c.GlobalString("addr"), opts...)
Expand Down
5 changes: 3 additions & 2 deletions control/gateway/gateway.go
Original file line number Diff line number Diff line change
Expand Up @@ -59,8 +59,9 @@ func (gwf *GatewayForwarder) lookupForwarder(ctx context.Context) (gateway.LLBBr
return nil, errors.New("no buildid found in context")
}

ctx, cancel := context.WithTimeout(ctx, 3*time.Second)
defer cancel()
ctx, cancel := context.WithCancelCause(ctx)
ctx, _ = context.WithTimeoutCause(ctx, 3*time.Second, errors.WithStack(context.DeadlineExceeded))
defer cancel(errors.WithStack(context.Canceled))

go func() {
<-ctx.Done()
Expand Down
9 changes: 5 additions & 4 deletions executor/containerdexecutor/executor.go
Original file line number Diff line number Diff line change
Expand Up @@ -371,21 +371,22 @@ func (w *containerdExecutor) runProcess(ctx context.Context, p containerd.Proces
}
}()

var cancel func()
var cancel func(error)
var killCtxDone <-chan struct{}
ctxDone := ctx.Done()
for {
select {
case <-ctxDone:
ctxDone = nil
var killCtx context.Context
killCtx, cancel = context.WithTimeout(context.Background(), 10*time.Second)
killCtx, cancel = context.WithCancelCause(context.Background())
killCtx, _ = context.WithTimeoutCause(killCtx, 10*time.Second, errors.WithStack(context.DeadlineExceeded))
killCtxDone = killCtx.Done()
p.Kill(killCtx, syscall.SIGKILL)
io.Cancel()
case status := <-statusCh:
if cancel != nil {
cancel()
cancel(errors.WithStack(context.Canceled))
}
trace.SpanFromContext(ctx).AddEvent(
"Container exited",
Expand All @@ -411,7 +412,7 @@ func (w *containerdExecutor) runProcess(ctx context.Context, p containerd.Proces
return nil
case <-killCtxDone:
if cancel != nil {
cancel()
cancel(errors.WithStack(context.Canceled))
}
io.Cancel()
return errors.Errorf("failed to kill process on cancel")
Expand Down
18 changes: 10 additions & 8 deletions executor/runcexecutor/executor.go
Original file line number Diff line number Diff line change
Expand Up @@ -532,8 +532,9 @@ func (k procKiller) Kill(ctx context.Context) (err error) {

// this timeout is generally a no-op, the Kill ctx should already have a
// shorter timeout but here as a fail-safe for future refactoring.
ctx, timeout := context.WithTimeout(ctx, 10*time.Second)
defer timeout()
ctx, cancel := context.WithCancelCause(ctx)
ctx, _ = context.WithTimeoutCause(ctx, 10*time.Second, errors.WithStack(context.DeadlineExceeded))
defer cancel(errors.WithStack(context.Canceled))

if k.pidfile == "" {
// for `runc run` process we use `runc kill` to terminate the process
Expand Down Expand Up @@ -615,17 +616,17 @@ func runcProcessHandle(ctx context.Context, killer procKiller) (*procHandle, con
for {
select {
case <-ctx.Done():
killCtx, timeout := context.WithTimeout(context.Background(), 7*time.Second)
killCtx, timeout := context.WithCancelCause(context.Background())
killCtx, _ = context.WithTimeoutCause(killCtx, 7*time.Second, errors.WithStack(context.DeadlineExceeded))
if err := p.killer.Kill(killCtx); err != nil {
select {
case <-killCtx.Done():
timeout()
cancel(errors.WithStack(context.Cause(ctx)))
return
default:
}
}
timeout()
timeout(errors.WithStack(context.Canceled))
select {
case <-time.After(50 * time.Millisecond):
case <-p.ended:
Expand Down Expand Up @@ -673,10 +674,11 @@ func (p *procHandle) WaitForReady(ctx context.Context) error {
// We wait for up to 10s for the runc pid to be reported. If the started
// callback is non-nil it will be called after receiving the pid.
func (p *procHandle) WaitForStart(ctx context.Context, startedCh <-chan int, started func()) error {
startedCtx, timeout := context.WithTimeout(ctx, 10*time.Second)
defer timeout()
ctx, cancel := context.WithCancelCause(ctx)
ctx, _ = context.WithTimeoutCause(ctx, 10*time.Second, errors.WithStack(context.DeadlineExceeded))
defer cancel(errors.WithStack(context.Canceled))
select {
case <-startedCtx.Done():
case <-ctx.Done():
return errors.New("go-runc started message never received")
case runcPid, ok := <-startedCh:
if !ok {
Expand Down
5 changes: 3 additions & 2 deletions exporter/local/export.go
Original file line number Diff line number Diff line change
Expand Up @@ -61,8 +61,9 @@ func (e *localExporter) Config() *exporter.Config {
}

func (e *localExporterInstance) Export(ctx context.Context, inp *exporter.Source, sessionID string) (map[string]string, exporter.DescriptorReference, error) {
timeoutCtx, cancel := context.WithTimeout(ctx, 5*time.Second)
defer cancel()
timeoutCtx, cancel := context.WithCancelCause(ctx)
timeoutCtx, _ = context.WithTimeoutCause(timeoutCtx, 5*time.Second, errors.WithStack(context.DeadlineExceeded))
defer cancel(errors.WithStack(context.Canceled))

if e.opts.Epoch == nil {
if tm, ok, err := epoch.ParseSource(inp); err != nil {
Expand Down
5 changes: 3 additions & 2 deletions exporter/oci/export.go
Original file line number Diff line number Diff line change
Expand Up @@ -198,8 +198,9 @@ func (e *imageExporterInstance) Export(ctx context.Context, src *exporter.Source
return nil, nil, errors.Errorf("invalid variant %q", e.opt.Variant)
}

timeoutCtx, cancel := context.WithTimeout(ctx, 5*time.Second)
defer cancel()
timeoutCtx, cancel := context.WithCancelCause(ctx)
timeoutCtx, _ = context.WithTimeoutCause(timeoutCtx, 5*time.Second, errors.WithStack(context.DeadlineExceeded))
defer cancel(errors.WithStack(context.Canceled))

caller, err := e.opt.SessionManager.Get(timeoutCtx, sessionID, false)
if err != nil {
Expand Down
5 changes: 3 additions & 2 deletions exporter/tar/export.go
Original file line number Diff line number Diff line change
Expand Up @@ -143,8 +143,9 @@ func (e *localExporterInstance) Export(ctx context.Context, inp *exporter.Source
fs = d.FS
}

timeoutCtx, cancel := context.WithTimeout(ctx, 5*time.Second)
defer cancel()
timeoutCtx, cancel := context.WithCancelCause(ctx)
timeoutCtx, _ = context.WithTimeoutCause(timeoutCtx, 5*time.Second, errors.WithStack(context.DeadlineExceeded))
defer cancel(errors.WithStack(context.Canceled))

caller, err := e.opt.SessionManager.Get(timeoutCtx, sessionID, false)
if err != nil {
Expand Down
5 changes: 3 additions & 2 deletions frontend/dockerfile/dockerfile_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2692,8 +2692,9 @@ COPY . .
fstest.CreateFile(".dockerignore", []byte("!\n"), 0600),
)

ctx, cancel := context.WithTimeout(sb.Context(), 15*time.Second)
defer cancel()
ctx, cancel := context.WithCancelCause(sb.Context())
ctx, _ = context.WithTimeoutCause(ctx, 15*time.Second, errors.WithStack(context.DeadlineExceeded))
defer cancel(errors.WithStack(context.Canceled))

c, err := client.New(ctx, sb.Address())
require.NoError(t, err)
Expand Down
5 changes: 3 additions & 2 deletions frontend/gateway/grpcclient/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -43,8 +43,9 @@ type GrpcClient interface {
}

func New(ctx context.Context, opts map[string]string, session, product string, c pb.LLBBridgeClient, w []client.WorkerInfo) (GrpcClient, error) {
pingCtx, pingCancel := context.WithTimeout(ctx, 15*time.Second)
defer pingCancel()
pingCtx, pingCancel := context.WithCancelCause(ctx)
pingCtx, _ = context.WithTimeoutCause(pingCtx, 15*time.Second, errors.WithStack(context.DeadlineExceeded))
defer pingCancel(errors.WithStack(context.Canceled))
resp, err := c.Ping(pingCtx, &pb.PingRequest{})
if err != nil {
return nil, err
Expand Down
5 changes: 3 additions & 2 deletions session/group.go
Original file line number Diff line number Diff line change
Expand Up @@ -72,8 +72,9 @@ func (sm *Manager) Any(ctx context.Context, g Group, f func(context.Context, str
return errors.Errorf("no active sessions")
}

timeoutCtx, cancel := context.WithTimeout(ctx, 5*time.Second)
defer cancel()
timeoutCtx, cancel := context.WithCancelCause(ctx)
timeoutCtx, _ = context.WithTimeoutCause(timeoutCtx, 5*time.Second, errors.WithStack(context.DeadlineExceeded))
defer cancel(errors.WithStack(context.Canceled))
c, err := sm.Get(timeoutCtx, id, false)
if err != nil {
lastErr = err
Expand Down
6 changes: 4 additions & 2 deletions session/grpc.go
Original file line number Diff line number Diff line change
Expand Up @@ -104,9 +104,11 @@ func monitorHealth(ctx context.Context, cc *grpc.ClientConn, cancelConn func(err
healthcheckStart := time.Now()

timeout := time.Duration(math.Max(float64(defaultHealthcheckDuration), float64(lastHealthcheckDuration)*1.5))
ctx, cancel := context.WithTimeout(ctx, timeout)

ctx, cancel := context.WithCancelCause(ctx)
ctx, _ = context.WithTimeoutCause(ctx, timeout, errors.WithStack(context.DeadlineExceeded))
_, err := healthClient.Check(ctx, &grpc_health_v1.HealthCheckRequest{})
cancel()
cancel(errors.WithStack(context.Canceled))

lastHealthcheckDuration = time.Since(healthcheckStart)
logFields := logrus.Fields{
Expand Down
5 changes: 3 additions & 2 deletions solver/jobs.go
Original file line number Diff line number Diff line change
Expand Up @@ -488,8 +488,9 @@ func (jl *Solver) NewJob(id string) (*Job, error) {
}

func (jl *Solver) Get(id string) (*Job, error) {
ctx, cancel := context.WithTimeout(context.Background(), 6*time.Second)
defer cancel()
ctx, cancel := context.WithCancelCause(context.Background())
ctx, _ = context.WithTimeoutCause(ctx, 6*time.Second, errors.WithStack(context.DeadlineExceeded))
defer cancel(errors.WithStack(context.Canceled))

go func() {
<-ctx.Done()
Expand Down
5 changes: 3 additions & 2 deletions solver/llbsolver/solver.go
Original file line number Diff line number Diff line change
Expand Up @@ -195,8 +195,9 @@ func (s *Solver) recordBuildHistory(ctx context.Context, id string, req frontend
}
}

ctx, cancel := context.WithTimeout(context.Background(), 20*time.Second)
defer cancel()
ctx, cancel := context.WithCancelCause(context.Background())
ctx, _ = context.WithTimeoutCause(ctx, 20*time.Second, errors.WithStack(context.DeadlineExceeded))
defer cancel(errors.WithStack(context.Canceled))

var mu sync.Mutex
ch := make(chan *client.SolveStatus)
Expand Down
5 changes: 3 additions & 2 deletions source/containerimage/ocilayout.go
Original file line number Diff line number Diff line change
Expand Up @@ -123,8 +123,9 @@ func (r *ociLayoutResolver) info(ctx context.Context, ref reference.Spec) (conte

func (r *ociLayoutResolver) withCaller(ctx context.Context, f func(context.Context, session.Caller) error) error {
if r.store.SessionID != "" {
timeoutCtx, cancel := context.WithTimeout(ctx, 5*time.Second)
defer cancel()
timeoutCtx, cancel := context.WithCancelCause(ctx)
timeoutCtx, _ = context.WithTimeoutCause(timeoutCtx, 5*time.Second, errors.WithStack(context.DeadlineExceeded))
defer cancel(errors.WithStack(context.Canceled))

caller, err := r.sm.Get(timeoutCtx, r.store.SessionID, false)
if err != nil {
Expand Down
5 changes: 3 additions & 2 deletions source/local/source.go
Original file line number Diff line number Diff line change
Expand Up @@ -141,8 +141,9 @@ func (ls *localSourceHandler) Snapshot(ctx context.Context, g session.Group) (ca
return ls.snapshotWithAnySession(ctx, g)
}

timeoutCtx, cancel := context.WithTimeout(ctx, 5*time.Second)
defer cancel()
timeoutCtx, cancel := context.WithCancelCause(ctx)
timeoutCtx, _ = context.WithTimeoutCause(timeoutCtx, 5*time.Second, errors.WithStack(context.DeadlineExceeded))
defer cancel(errors.WithStack(context.Canceled))

caller, err := ls.sm.Get(timeoutCtx, sessionID, false)
if err != nil {
Expand Down
5 changes: 3 additions & 2 deletions util/testutil/integration/registry.go
Original file line number Diff line number Diff line change
Expand Up @@ -67,8 +67,9 @@ http:
}
deferF.Append(stop)

ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second)
defer cancel()
ctx, cancel := context.WithCancelCause(context.Background())
ctx, _ = context.WithTimeoutCause(ctx, 5*time.Second, errors.WithStack(context.DeadlineExceeded))
defer cancel(errors.WithStack(context.Canceled))
url, err = detectPort(ctx, rc)
if err != nil {
return "", nil, err
Expand Down

0 comments on commit 86ad76e

Please sign in to comment.