Skip to content

Commit

Permalink
builder-next: fix missing lock in ensurelayer
Browse files Browse the repository at this point in the history
When this was called concurrently from the moby image
exporter there could be a data race where a layer was
written to the refs map when it was already there.

In that case the reference count got mixed up and on
release only one of these layers was actually released.

Signed-off-by: Tonis Tiigi <tonistiigi@gmail.com>
(cherry picked from commit 37545cc)
Signed-off-by: Paweł Gronowski <pawel.gronowski@docker.com>
  • Loading branch information
tonistiigi authored and vvoland committed Mar 7, 2024
1 parent 061aa95 commit d250e13
Show file tree
Hide file tree
Showing 2 changed files with 14 additions and 8 deletions.
3 changes: 3 additions & 0 deletions builder/builder-next/adapters/snapshot/layer.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,9 @@ func (s *snapshotter) GetDiffIDs(ctx context.Context, key string) ([]layer.DiffI
}

func (s *snapshotter) EnsureLayer(ctx context.Context, key string) ([]layer.DiffID, error) {
s.layerCreateLocker.Lock(key)
defer s.layerCreateLocker.Unlock(key)

diffIDs, err := s.GetDiffIDs(ctx, key)
if err != nil {
return nil, err
Expand Down
19 changes: 11 additions & 8 deletions builder/builder-next/adapters/snapshot/snapshot.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ import (
"github.com/moby/buildkit/identity"
"github.com/moby/buildkit/snapshot"
"github.com/moby/buildkit/util/leaseutil"
"github.com/moby/locker"
"github.com/opencontainers/go-digest"
"github.com/pkg/errors"
bolt "go.etcd.io/bbolt"
Expand Down Expand Up @@ -51,10 +52,11 @@ type checksumCalculator interface {
type snapshotter struct {
opt Opt

refs map[string]layer.Layer
db *bolt.DB
mu sync.Mutex
reg graphIDRegistrar
refs map[string]layer.Layer
db *bolt.DB
mu sync.Mutex
reg graphIDRegistrar
layerCreateLocker *locker.Locker
}

// NewSnapshotter creates a new snapshotter
Expand All @@ -71,10 +73,11 @@ func NewSnapshotter(opt Opt, prevLM leases.Manager, ns string) (snapshot.Snapsho
}

s := &snapshotter{
opt: opt,
db: db,
refs: map[string]layer.Layer{},
reg: reg,
opt: opt,
db: db,
refs: map[string]layer.Layer{},
reg: reg,
layerCreateLocker: locker.New(),
}

slm := newLeaseManager(s, prevLM)
Expand Down

0 comments on commit d250e13

Please sign in to comment.