Skip to content

Commit

Permalink
Merge pull request #5031 from tonistiigi/context-withoutcancel
Browse files Browse the repository at this point in the history
ensure context.WithoutCancel in defer funcs
  • Loading branch information
tonistiigi authored Jun 18, 2024
2 parents 12f04ef + 4103099 commit ff1674a
Show file tree
Hide file tree
Showing 20 changed files with 56 additions and 43 deletions.
3 changes: 2 additions & 1 deletion cache/blobs_linux.go
Original file line number Diff line number Diff line change
Expand Up @@ -46,12 +46,13 @@ func (sr *immutableRef) tryComputeOverlayBlob(ctx context.Context, lower, upper

defer func() {
if cw != nil {
ctx = context.WithoutCancel(ctx)
// after commit success cw will be set to nil, if cw isn't nil, error
// happened before commit, we should abort this ingest, and because the
// error may incured by ctx cancel, use a new context here. And since
// cm.Close will unlock this ref in the content store, we invoke abort
// to remove the ingest root in advance.
if aerr := sr.cm.ContentStore.Abort(context.Background(), ref); aerr != nil {
if aerr := sr.cm.ContentStore.Abort(ctx, ref); aerr != nil {
bklog.G(ctx).WithError(aerr).Warnf("failed to abort writer %q", ref)
}
if cerr := cw.Close(); cerr != nil {
Expand Down
26 changes: 15 additions & 11 deletions cache/manager.go
Original file line number Diff line number Diff line change
Expand Up @@ -168,7 +168,7 @@ func (cm *cacheManager) GetByBlob(ctx context.Context, desc ocispecs.Descriptor,
releaseParent := false
defer func() {
if releaseParent || rerr != nil && p != nil {
p.Release(context.TODO())
p.Release(context.WithoutCancel(ctx))
}
}()

Expand Down Expand Up @@ -242,7 +242,8 @@ func (cm *cacheManager) GetByBlob(ctx context.Context, desc ocispecs.Descriptor,

defer func() {
if rerr != nil {
if err := cm.LeaseManager.Delete(context.TODO(), leases.Lease{
ctx := context.WithoutCancel(ctx)
if err := cm.LeaseManager.Delete(ctx, leases.Lease{
ID: l.ID,
}); err != nil {
bklog.G(ctx).Errorf("failed to remove lease: %+v", err)
Expand Down Expand Up @@ -423,7 +424,7 @@ func (cm *cacheManager) getRecord(ctx context.Context, id string, opts ...RefOpt
}
defer func() {
if retErr != nil {
parents.release(context.TODO())
parents.release(context.WithoutCancel(ctx))
}
}()

Expand Down Expand Up @@ -513,7 +514,7 @@ func (cm *cacheManager) getRecord(ctx context.Context, id string, opts ...RefOpt
func (cm *cacheManager) parentsOf(ctx context.Context, md *cacheMetadata, opts ...RefOption) (ps parentRefs, rerr error) {
defer func() {
if rerr != nil {
ps.release(context.TODO())
ps.release(context.WithoutCancel(ctx))
}
}()
if parentID := md.getParent(); parentID != "" {
Expand Down Expand Up @@ -580,7 +581,7 @@ func (cm *cacheManager) New(ctx context.Context, s ImmutableRef, sess session.Gr

defer func() {
if err != nil && parent != nil {
parent.Release(context.TODO())
parent.Release(context.WithoutCancel(ctx))
}
}()

Expand All @@ -597,7 +598,8 @@ func (cm *cacheManager) New(ctx context.Context, s ImmutableRef, sess session.Gr

defer func() {
if err != nil {
if err := cm.LeaseManager.Delete(context.TODO(), leases.Lease{
ctx := context.WithoutCancel(ctx)
if err := cm.LeaseManager.Delete(ctx, leases.Lease{
ID: l.ID,
}); err != nil {
bklog.G(ctx).Errorf("failed to remove lease: %+v", err)
Expand Down Expand Up @@ -705,7 +707,7 @@ func (cm *cacheManager) Merge(ctx context.Context, inputParents []ImmutableRef,
dhs := make(map[digest.Digest]*DescHandler)
defer func() {
if rerr != nil {
parents.release(context.TODO())
parents.release(context.WithoutCancel(ctx))
}
}()
for _, inputParent := range inputParents {
Expand Down Expand Up @@ -798,7 +800,8 @@ func (cm *cacheManager) createMergeRef(ctx context.Context, parents parentRefs,
}
defer func() {
if rerr != nil {
if err := cm.LeaseManager.Delete(context.TODO(), leases.Lease{
ctx := context.WithoutCancel(ctx)
if err := cm.LeaseManager.Delete(ctx, leases.Lease{
ID: l.ID,
}); err != nil {
bklog.G(ctx).Errorf("failed to remove lease: %+v", err)
Expand Down Expand Up @@ -834,7 +837,7 @@ func (cm *cacheManager) Diff(ctx context.Context, lower, upper ImmutableRef, pg
dhs := make(map[digest.Digest]*DescHandler)
defer func() {
if rerr != nil {
parents.release(context.TODO())
parents.release(context.WithoutCancel(ctx))
}
}()
for i, inputParent := range []ImmutableRef{lower, upper} {
Expand Down Expand Up @@ -889,7 +892,7 @@ func (cm *cacheManager) Diff(ctx context.Context, lower, upper ImmutableRef, pg
mergeParents := parentRefs{mergeParents: make([]*immutableRef, len(upperLayers)-len(lowerLayers))}
defer func() {
if rerr != nil {
mergeParents.release(context.TODO())
mergeParents.release(context.WithoutCancel(ctx))
}
}()
for i := len(lowerLayers); i < len(upperLayers); i++ {
Expand Down Expand Up @@ -954,7 +957,8 @@ func (cm *cacheManager) createDiffRef(ctx context.Context, parents parentRefs, d
}
defer func() {
if rerr != nil {
if err := cm.LeaseManager.Delete(context.TODO(), leases.Lease{
ctx := context.WithoutCancel(ctx)
if err := cm.LeaseManager.Delete(ctx, leases.Lease{
ID: l.ID,
}); err != nil {
bklog.G(ctx).Errorf("failed to remove lease: %+v", err)
Expand Down
2 changes: 1 addition & 1 deletion cache/manager_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1168,7 +1168,7 @@ func TestLoopLeaseContent(t *testing.T) {
allRefs := []ImmutableRef{ref}
defer func() {
for _, ref := range allRefs {
ref.Release(ctx)
ref.Release(context.WithoutCancel(ctx))
}
}()
var chain []ocispecs.Descriptor
Expand Down
2 changes: 2 additions & 0 deletions cache/refs.go
Original file line number Diff line number Diff line change
Expand Up @@ -1044,6 +1044,7 @@ func (sr *immutableRef) withRemoteSnapshotLabelsStargzMode(ctx context.Context,
return errors.Wrapf(err, "failed to add tmp remote labels for remote snapshot")
}
defer func() {
ctx := context.WithoutCancel(ctx)
for k := range info.Labels {
info.Labels[k] = "" // Remove labels appended in this call
}
Expand Down Expand Up @@ -1105,6 +1106,7 @@ func (sr *immutableRef) prepareRemoteSnapshotsStargzMode(ctx context.Context, s
info, err := r.cm.Snapshotter.Stat(ctx, snapshotID)
if err == nil { // usable as remote snapshot without unlazying.
defer func() {
ctx := context.WithoutCancel(ctx)
// Remove tmp labels appended in this func
if info.Labels != nil {
for k := range tmpLabels {
Expand Down
2 changes: 1 addition & 1 deletion cache/remote.go
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ func (sr *immutableRef) GetRemotes(ctx context.Context, createIfNeeded bool, ref
if err != nil {
return nil, err
}
defer done(ctx)
defer done(context.WithoutCancel(ctx))

// fast path if compression variants aren't required
// NOTE: compressionopt is applied only to *newly created layers* if Force != true.
Expand Down
4 changes: 2 additions & 2 deletions client/build_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -432,13 +432,13 @@ func testClientGatewayContainerExecPipe(t *testing.T, sb integration.Sandbox) {
Args: []string{"sleep", "10"},
})
if err != nil {
ctr.Release(ctx)
ctr.Release(context.WithoutCancel(ctx))
return nil, err
}

defer func() {
// cancel pid1
ctr.Release(ctx)
ctr.Release(context.WithoutCancel(ctx))
pid1.Wait()
}()

Expand Down
4 changes: 2 additions & 2 deletions executor/containerdexecutor/executor.go
Original file line number Diff line number Diff line change
Expand Up @@ -166,7 +166,7 @@ func (w *containerdExecutor) Run(ctx context.Context, id string, root executor.M
}

defer func() {
if err1 := container.Delete(context.TODO()); err == nil && err1 != nil {
if err1 := container.Delete(context.WithoutCancel(ctx)); err == nil && err1 != nil {
err = errors.Wrapf(err1, "failed to delete container %s", id)
}
}()
Expand All @@ -190,7 +190,7 @@ func (w *containerdExecutor) Run(ctx context.Context, id string, root executor.M
}

defer func() {
if _, err1 := task.Delete(context.TODO(), containerd.WithProcessKill); err == nil && err1 != nil {
if _, err1 := task.Delete(context.WithoutCancel(ctx), containerd.WithProcessKill); err == nil && err1 != nil {
err = errors.Wrapf(err1, "failed to delete task %s", id)
}
}()
Expand Down
2 changes: 1 addition & 1 deletion executor/runcexecutor/executor.go
Original file line number Diff line number Diff line change
Expand Up @@ -237,7 +237,7 @@ func (w *runcExecutor) Run(ctx context.Context, id string, root executor.Mount,
}
defer mount.Unmount(rootFSPath, 0)

defer executor.MountStubsCleaner(ctx, rootFSPath, mounts, meta.RemoveMountStubsRecursive)()
defer executor.MountStubsCleaner(context.WithoutCancel(ctx), rootFSPath, mounts, meta.RemoveMountStubsRecursive)()

uid, gid, sgids, err := oci.GetUser(rootFSPath, meta.User)
if err != nil {
Expand Down
2 changes: 1 addition & 1 deletion exporter/containerimage/export.go
Original file line number Diff line number Diff line change
Expand Up @@ -226,7 +226,7 @@ func (e *imageExporterInstance) Export(ctx context.Context, src *exporter.Source
}
defer func() {
if descref == nil {
done(context.TODO())
done(context.WithoutCancel(ctx))
}
}()

Expand Down
2 changes: 1 addition & 1 deletion exporter/oci/export.go
Original file line number Diff line number Diff line change
Expand Up @@ -155,7 +155,7 @@ func (e *imageExporterInstance) Export(ctx context.Context, src *exporter.Source
}
defer func() {
if descref == nil {
done(context.TODO())
done(context.WithoutCancel(ctx))
}
}()

Expand Down
8 changes: 5 additions & 3 deletions frontend/gateway/gateway.go
Original file line number Diff line number Diff line change
Expand Up @@ -155,8 +155,9 @@ func (gf *gatewayFrontend) Solve(ctx context.Context, llbBridge frontend.Fronten
return nil, err
}
defer func() {
ctx := context.WithoutCancel(ctx)
devRes.EachRef(func(ref solver.ResultProxy) error {
return ref.Release(context.TODO())
return ref.Release(ctx)
})
}()
if devRes.Ref == nil {
Expand Down Expand Up @@ -248,8 +249,9 @@ func (gf *gatewayFrontend) Solve(ctx context.Context, llbBridge frontend.Fronten
return nil, err
}
defer func() {
ctx := context.WithoutCancel(ctx)
res.EachRef(func(ref solver.ResultProxy) error {
return ref.Release(context.TODO())
return ref.Release(ctx)
})
}()
if res.Ref == nil {
Expand Down Expand Up @@ -1153,7 +1155,7 @@ func (lbf *llbBridgeForwarder) NewContainer(ctx context.Context, in *pb.NewConta
}
defer func() {
if err != nil {
ctr.Release(ctx) // ensure release on error
ctr.Release(context.WithoutCancel(ctx)) // ensure release on error
}
}()

Expand Down
3 changes: 2 additions & 1 deletion solver/combinedcache.go
Original file line number Diff line number Diff line change
Expand Up @@ -73,11 +73,12 @@ func (cm *combinedCacheManager) Load(ctx context.Context, rec *CacheRecord) (res
return nil, err
}
defer func() {
ctx := context.WithoutCancel(ctx)
for i, res := range results {
if err == nil && i == 0 {
continue
}
res.Result.Release(context.TODO())
res.Result.Release(ctx)
}
}()
if rec.cacheManager != cm.main && cm.main != nil {
Expand Down
3 changes: 2 additions & 1 deletion solver/llbsolver/file/refmanager.go
Original file line number Diff line number Diff line change
Expand Up @@ -46,10 +46,11 @@ func (rm *RefManager) Prepare(ctx context.Context, ref fileoptypes.Ref, readonly
}
defer func() {
if rerr != nil {
ctx := context.WithoutCancel(ctx)
if err := mr.SetCachePolicyDefault(); err != nil {
bklog.G(ctx).Errorf("failed to reset FileOp mutable ref cachepolicy: %v", err)
}
mr.Release(context.TODO())
mr.Release(ctx)
}
}()
m, err := mr.Mount(ctx, readonly, g)
Expand Down
6 changes: 3 additions & 3 deletions solver/llbsolver/history.go
Original file line number Diff line number Diff line change
Expand Up @@ -133,7 +133,7 @@ func (h *HistoryQueue) migrateV2() error {
if err != nil {
return err
}
defer release(ctx)
defer release(context.WithoutCancel(ctx))
return b.ForEach(func(key, dt []byte) error {
recs, err := h.opt.LeaseManager.ListResources(ctx, leases.Lease{ID: h.leaseID(string(key))})
if err != nil {
Expand Down Expand Up @@ -518,7 +518,7 @@ func (h *HistoryQueue) update(ctx context.Context, rec controlapi.BuildHistoryRe

defer func() {
if err != nil && created {
h.hLeaseManager.Delete(ctx, l)
h.hLeaseManager.Delete(context.WithoutCancel(ctx), l)
}
}()

Expand Down Expand Up @@ -598,7 +598,7 @@ func (h *HistoryQueue) OpenBlobWriter(ctx context.Context, mt string) (_ *Writer

defer func() {
if err != nil {
h.hLeaseManager.Delete(ctx, l)
h.hLeaseManager.Delete(context.WithoutCancel(ctx), l)
}
}()

Expand Down
6 changes: 4 additions & 2 deletions solver/llbsolver/ops/file.go
Original file line number Diff line number Diff line change
Expand Up @@ -354,7 +354,7 @@ func (s *FileOpSolver) Solve(ctx context.Context, inputs []fileoptypes.Ref, acti
defer func() {
for _, in := range s.ins {
if in.ref == nil && in.mount != nil {
in.mount.Release(context.TODO())
in.mount.Release(context.WithoutCancel(ctx))
}
}
}()
Expand Down Expand Up @@ -432,6 +432,7 @@ func (s *FileOpSolver) getInput(ctx context.Context, idx int, inputs []fileoptyp

defer func() {
if err != nil && inpMount != nil {
ctx := context.WithoutCancel(ctx)
inputRes := make([]solver.Result, len(inputs))
for i, input := range inputs {
inputRes[i] = worker.NewWorkerRefResult(input.(cache.ImmutableRef), s.w)
Expand All @@ -458,8 +459,9 @@ func (s *FileOpSolver) getInput(ctx context.Context, idx int, inputs []fileoptyp

err = errdefs.WithExecErrorWithContext(ctx, err, inputRes, outputRes)
}
ctx := context.WithoutCancel(ctx)
for _, m := range toRelease {
m.Release(context.TODO())
m.Release(ctx)
}
}()

Expand Down
2 changes: 1 addition & 1 deletion solver/llbsolver/solver.go
Original file line number Diff line number Diff line change
Expand Up @@ -507,7 +507,7 @@ func (s *Solver) Solve(ctx context.Context, id string, sessionID string, req fro
if err := s.gatewayForwarder.RegisterBuild(ctx, id, fwd); err != nil {
return nil, err
}
defer s.gatewayForwarder.UnregisterBuild(ctx, id)
defer s.gatewayForwarder.UnregisterBuild(context.WithoutCancel(ctx), id)
}

if !internal {
Expand Down
6 changes: 3 additions & 3 deletions source/containerimage/pull.go
Original file line number Diff line number Diff line change
Expand Up @@ -218,14 +218,14 @@ func (p *puller) Snapshot(ctx context.Context, g session.Group) (ir cache.Immuta
}
defer func() {
if p.releaseTmpLeases != nil {
p.releaseTmpLeases(context.TODO())
p.releaseTmpLeases(context.WithoutCancel(ctx))
}
}()

var current cache.ImmutableRef
defer func() {
if err != nil && current != nil {
current.Release(context.TODO())
current.Release(context.WithoutCancel(ctx))
}
}()

Expand Down Expand Up @@ -255,7 +255,7 @@ func (p *puller) Snapshot(ctx context.Context, g session.Group) (ir cache.Immuta
if err != nil {
return nil, err
}
defer done(ctx)
defer done(context.WithoutCancel(ctx))

if _, err := p.PullManifests(ctx, getResolver); err != nil {
return nil, err
Expand Down
4 changes: 2 additions & 2 deletions source/git/source.go
Original file line number Diff line number Diff line change
Expand Up @@ -497,7 +497,7 @@ func (gs *gitSourceHandler) Snapshot(ctx context.Context, g session.Group) (out

defer func() {
if retErr != nil && checkoutRef != nil {
checkoutRef.Release(context.TODO())
checkoutRef.Release(context.WithoutCancel(ctx))
}
}()

Expand Down Expand Up @@ -646,7 +646,7 @@ func (gs *gitSourceHandler) Snapshot(ctx context.Context, g session.Group) (out

defer func() {
if retErr != nil {
snap.Release(context.TODO())
snap.Release(context.WithoutCancel(ctx))
}
}()

Expand Down
Loading

0 comments on commit ff1674a

Please sign in to comment.