Skip to content

Commit

Permalink
solver: fix possible concurrent map access on cache export
Browse files Browse the repository at this point in the history
Signed-off-by: Tonis Tiigi <tonistiigi@gmail.com>
(cherry picked from commit e99bfa9)
  • Loading branch information
tonistiigi authored and crazy-max committed Nov 29, 2023
1 parent f4f4d2a commit f83447b
Show file tree
Hide file tree
Showing 3 changed files with 35 additions and 25 deletions.
4 changes: 3 additions & 1 deletion solver/cachekey.go
Original file line number Diff line number Diff line change
Expand Up @@ -55,15 +55,17 @@ func (ck *CacheKey) Output() Index {
}

func (ck *CacheKey) clone() *CacheKey {
ck.mu.RLock()
nk := &CacheKey{
ID: ck.ID,
digest: ck.digest,
vtx: ck.vtx,
output: ck.output,
ids: map[*cacheManager]string{},
ids: make(map[*cacheManager]string, len(ck.ids)),
}
for cm, id := range ck.ids {
nk.ids[cm] = id
}
ck.mu.RUnlock()
return nk
}
43 changes: 25 additions & 18 deletions solver/combinedcache.go
Original file line number Diff line number Diff line change
Expand Up @@ -101,34 +101,41 @@ func (cm *combinedCacheManager) Save(key *CacheKey, s Result, createdAt time.Tim
}

func (cm *combinedCacheManager) Records(ctx context.Context, ck *CacheKey) ([]*CacheRecord, error) {
ck.mu.RLock()
if len(ck.ids) == 0 {
ck.mu.RUnlock()
return nil, errors.Errorf("no results")
}

cms := make([]*cacheManager, 0, len(ck.ids))
for cm := range ck.ids {
cms = append(cms, cm)
}
ck.mu.RUnlock()

records := map[string]*CacheRecord{}
var mu sync.Mutex

eg, _ := errgroup.WithContext(context.TODO())
for c := range ck.ids {
func(c *cacheManager) {
eg.Go(func() error {
recs, err := c.Records(ctx, ck)
if err != nil {
return err
}
mu.Lock()
for _, rec := range recs {
if _, ok := records[rec.ID]; !ok || c == cm.main {
if c == cm.main {
rec.Priority = 1
}
records[rec.ID] = rec
for _, c := range cms {
c := c
eg.Go(func() error {
recs, err := c.Records(ctx, ck)
if err != nil {
return err
}
mu.Lock()
for _, rec := range recs {
if _, ok := records[rec.ID]; !ok || c == cm.main {
if c == cm.main {
rec.Priority = 1
}
records[rec.ID] = rec
}
mu.Unlock()
return nil
})
}(c)
}
mu.Unlock()
return nil
})
}

if err := eg.Wait(); err != nil {
Expand Down
13 changes: 7 additions & 6 deletions solver/exporter.go
Original file line number Diff line number Diff line change
Expand Up @@ -85,8 +85,9 @@ func (e *exporter) ExportTo(ctx context.Context, t CacheExporterTarget, opt Cach
r CacheExporterRecord
selector digest.Digest
}
k := e.k.clone() // protect against *CacheKey internal ids mutation from other exports

recKey := rootKey(e.k.Digest(), e.k.Output())
recKey := rootKey(k.Digest(), k.Output())
rec := t.Add(recKey)
allRec := []CacheExporterRecord{rec}

Expand All @@ -97,7 +98,7 @@ func (e *exporter) ExportTo(ctx context.Context, t CacheExporterTarget, opt Cach
}

exportRecord := opt.ExportRoots
if len(e.k.Deps()) > 0 {
if len(deps) > 0 {
exportRecord = true
}

Expand Down Expand Up @@ -126,7 +127,7 @@ func (e *exporter) ExportTo(ctx context.Context, t CacheExporterTarget, opt Cach
if opt.CompressionOpt != nil {
for _, r := range remotes { // record all remaining remotes as well
rec := t.Add(recKey)
rec.AddResult(e.k.vtx, int(e.k.output), v.CreatedAt, r)
rec.AddResult(k.vtx, int(k.output), v.CreatedAt, r)
variants = append(variants, rec)
}
}
Expand All @@ -147,15 +148,15 @@ func (e *exporter) ExportTo(ctx context.Context, t CacheExporterTarget, opt Cach
if opt.CompressionOpt != nil {
for _, r := range remotes { // record all remaining remotes as well
rec := t.Add(recKey)
rec.AddResult(e.k.vtx, int(e.k.output), v.CreatedAt, r)
rec.AddResult(k.vtx, int(k.output), v.CreatedAt, r)
variants = append(variants, rec)
}
}
}

if remote != nil {
for _, rec := range allRec {
rec.AddResult(e.k.vtx, int(e.k.output), v.CreatedAt, remote)
rec.AddResult(k.vtx, int(k.output), v.CreatedAt, remote)
}
}
allRec = append(allRec, variants...)
Expand Down Expand Up @@ -198,7 +199,7 @@ func (e *exporter) ExportTo(ctx context.Context, t CacheExporterTarget, opt Cach
}
}

for cm, id := range e.k.ids {
for cm, id := range k.ids {
if _, err := addBacklinks(t, rec, cm, id, bkm); err != nil {
return nil, err
}
Expand Down

0 comments on commit f83447b

Please sign in to comment.