From 3660e5f9c8efe4bd8d2e61c684c950863abae2ee Mon Sep 17 00:00:00 2001 From: Tonis Tiigi Date: Tue, 2 Feb 2021 23:20:44 -0800 Subject: [PATCH] don't commit cache mounts on error Cache mount instances are shared between multiple vertextes/builds so if one of the cloned instance gets committed reference count will get corrupted as other parts of the code still see reference as mountable. Signed-off-by: Tonis Tiigi --- frontend/gateway/container.go | 2 ++ solver/llbsolver/ops/exec.go | 14 +++++++++----- 2 files changed, 11 insertions(+), 5 deletions(-) diff --git a/frontend/gateway/container.go b/frontend/gateway/container.go index 1f79bb68f..e124566da 100644 --- a/frontend/gateway/container.go +++ b/frontend/gateway/container.go @@ -127,6 +127,7 @@ type MountRef struct { type MountMutableRef struct { Ref cache.MutableRef MountIndex int + NoCommit bool } type MakeMutable func(m *opspb.Mount, ref cache.ImmutableRef) (cache.MutableRef, error) @@ -196,6 +197,7 @@ func PrepareMounts(ctx context.Context, mm *mounts.MountManager, cm cache.Manage p.Actives = append(p.Actives, MountMutableRef{ MountIndex: i, Ref: active, + NoCommit: true, }) if m.Output != opspb.SkipOutput && ref != nil { p.OutputRefs = append(p.OutputRefs, MountRef{ diff --git a/solver/llbsolver/ops/exec.go b/solver/llbsolver/ops/exec.go index 5ec45fafd..631d34d11 100644 --- a/solver/llbsolver/ops/exec.go +++ b/solver/llbsolver/ops/exec.go @@ -243,12 +243,16 @@ func (e *execOp) Exec(ctx context.Context, g session.Group, inputs []solver.Resu execMounts[p.OutputRefs[i].MountIndex] = res } for _, active := range p.Actives { - ref, cerr := active.Ref.Commit(ctx) - if cerr != nil { - err = errors.Wrapf(err, "error committing %s: %s", active.Ref.ID(), cerr) - continue + if active.NoCommit { + active.Ref.Release(context.TODO()) + } else { + ref, cerr := active.Ref.Commit(ctx) + if cerr != nil { + err = errors.Wrapf(err, "error committing %s: %s", active.Ref.ID(), cerr) + continue + } + execMounts[active.MountIndex] = worker.NewWorkerRefResult(ref, e.w) } - execMounts[active.MountIndex] = worker.NewWorkerRefResult(ref, e.w) } err = errdefs.WithExecError(err, execInputs, execMounts) } else {