Merge pull request #4452 from crazy-max/v0.12.4_cherry-picks

[0.12] cherry-picks 0.12.4
moby · Nov 29, 2023 · 833949d · 833949d
2 parents f4f4d2a + 020073b
commit 833949d
Show file tree

Hide file tree

Showing 5 changed files with 51 additions and 28 deletions.
diff --git a/cmd/buildkitd/debug.go b/cmd/buildkitd/debug.go
@@ -43,6 +43,10 @@ func setupDebugHandlers(addr string) error {
 		ReadHeaderTimeout: time.Minute,
 	}
 	bklog.L.Debugf("debug handlers listening at %s", addr)
-	go server.ListenAndServe()
+	go func() {
+		if err := server.Serve(l); err != nil {
+			bklog.L.Errorf("failed to serve debug handlers: %v", err)
+		}
+	}()
 	return nil
 }
diff --git a/solver/cachekey.go b/solver/cachekey.go
@@ -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
 }
diff --git a/solver/combinedcache.go b/solver/combinedcache.go
@@ -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 {

diff --git a/solver/exporter.go b/solver/exporter.go
@@ -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}
 
@@ -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
 	}
 
@@ -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)
 			}
 		}
@@ -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...)
@@ -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
 			}

diff --git a/solver/llbsolver/history.go b/solver/llbsolver/history.go
@@ -762,21 +762,30 @@ func (h *HistoryQueue) Listen(ctx context.Context, req *controlapi.BuildHistoryR
 		}()
 	}
 
+	// make a copy of events for active builds so we don't keep a lock during grpc send
+	actives := make([]*controlapi.BuildHistoryEvent, 0, len(h.active))
+
 	for _, e := range h.active {
 		if req.Ref != "" && e.Ref != req.Ref {
 			continue
 		}
 		if _, ok := h.deleted[e.Ref]; ok {
 			continue
 		}
-		sub.send(&controlapi.BuildHistoryEvent{
+		actives = append(actives, &controlapi.BuildHistoryEvent{
 			Type:   controlapi.BuildHistoryEventType_STARTED,
 			Record: e,
 		})
 	}
 
 	h.mu.Unlock()
 
+	for _, e := range actives {
+		if err := f(e); err != nil {
+			return err
+		}
+	}
+
 	if !req.ActiveOnly {
 		events := []*controlapi.BuildHistoryEvent{}
 		if err := h.opt.DB.View(func(tx *bolt.Tx) error {
@@ -810,7 +819,7 @@ func (h *HistoryQueue) Listen(ctx context.Context, req *controlapi.BuildHistoryR
 		}
 		h.mu.Unlock()
 		for _, e := range events {
-			if e.Record == nil {
+			if e == nil || e.Record == nil {
 				continue
 			}
 			if err := f(e); err != nil {