Browse code

vendor: update buildkit v0.6.4

full diff: https://github.com/moby/buildkit/compare/57e8ad52170d713233569f6d467e609d2b0f90c9...v0.6.4

- buildkit#1374 [v0.6] ops: fix deadlock on releasing shared mounts
- backport of buildkit#1355 ops: fix deadlock on releasing shared mounts
- fixes buildkit#1322 Deadlock on cache mounts

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>

Sebastiaan van Stijn authored on 2020/02/22 20:28:37
Showing 2 changed files
... ...
@@ -26,7 +26,7 @@ github.com/imdario/mergo                            7c29201646fa3de8506f70121347
26 26
 golang.org/x/sync                                   e225da77a7e68af35c70ccbf71af2b83e6acac3c
27 27
 
28 28
 # buildkit
29
-github.com/moby/buildkit                            57e8ad52170d713233569f6d467e609d2b0f90c9
29
+github.com/moby/buildkit                            ebcef1f69af0bbca077efa9a960a481e579a0e89 # v0.6.4
30 30
 github.com/tonistiigi/fsutil                        6c909ab392c173a4264ae1bfcbc0450b9aac0c7d
31 31
 github.com/grpc-ecosystem/grpc-opentracing          8e809c8a86450a29b90dcc9efbf062d0fe6d9746
32 32
 github.com/opentracing/opentracing-go               1361b9cd60be79c4c3a7fa9841b3c132e40066a7
... ...
@@ -56,7 +56,8 @@ type execOp struct {
56 56
 	platform  *pb.Platform
57 57
 	numInputs int
58 58
 
59
-	cacheMounts map[string]*cacheRefShare
59
+	cacheMounts   map[string]*cacheRefShare
60
+	cacheMountsMu sync.Mutex
60 61
 }
61 62
 
62 63
 func NewExecOp(v solver.Vertex, op *pb.Op_Exec, platform *pb.Platform, cm cache.Manager, sm *session.Manager, md *metadata.Store, exec executor.Executor, w worker.Worker) (solver.Op, error) {
... ...
@@ -221,56 +222,75 @@ func (e *execOp) getMountDeps() ([]dep, error) {
221 221
 }
222 222
 
223 223
 func (e *execOp) getRefCacheDir(ctx context.Context, ref cache.ImmutableRef, id string, m *pb.Mount, sharing pb.CacheSharingOpt) (mref cache.MutableRef, err error) {
224
+	g := &cacheRefGetter{
225
+		locker:          &e.cacheMountsMu,
226
+		cacheMounts:     e.cacheMounts,
227
+		cm:              e.cm,
228
+		md:              e.md,
229
+		globalCacheRefs: sharedCacheRefs,
230
+		name:            fmt.Sprintf("cached mount %s from exec %s", m.Dest, strings.Join(e.op.Meta.Args, " ")),
231
+	}
232
+	return g.getRefCacheDir(ctx, ref, id, sharing)
233
+}
234
+
235
+type cacheRefGetter struct {
236
+	locker          sync.Locker
237
+	cacheMounts     map[string]*cacheRefShare
238
+	cm              cache.Manager
239
+	md              *metadata.Store
240
+	globalCacheRefs *cacheRefs
241
+	name            string
242
+}
243
+
244
+func (g *cacheRefGetter) getRefCacheDir(ctx context.Context, ref cache.ImmutableRef, id string, sharing pb.CacheSharingOpt) (mref cache.MutableRef, err error) {
224 245
 	key := "cache-dir:" + id
225 246
 	if ref != nil {
226 247
 		key += ":" + ref.ID()
227 248
 	}
228
-	mu := CacheMountsLocker()
249
+	mu := g.locker
229 250
 	mu.Lock()
230 251
 	defer mu.Unlock()
231 252
 
232
-	if ref, ok := e.cacheMounts[key]; ok {
253
+	if ref, ok := g.cacheMounts[key]; ok {
233 254
 		return ref.clone(), nil
234 255
 	}
235 256
 	defer func() {
236 257
 		if err == nil {
237 258
 			share := &cacheRefShare{MutableRef: mref, refs: map[*cacheRef]struct{}{}}
238
-			e.cacheMounts[key] = share
259
+			g.cacheMounts[key] = share
239 260
 			mref = share.clone()
240 261
 		}
241 262
 	}()
242 263
 
243 264
 	switch sharing {
244 265
 	case pb.CacheSharingOpt_SHARED:
245
-		return sharedCacheRefs.get(key, func() (cache.MutableRef, error) {
246
-			return e.getRefCacheDirNoCache(ctx, key, ref, id, m, false)
266
+		return g.globalCacheRefs.get(key, func() (cache.MutableRef, error) {
267
+			return g.getRefCacheDirNoCache(ctx, key, ref, id, false)
247 268
 		})
248 269
 	case pb.CacheSharingOpt_PRIVATE:
249
-		return e.getRefCacheDirNoCache(ctx, key, ref, id, m, false)
270
+		return g.getRefCacheDirNoCache(ctx, key, ref, id, false)
250 271
 	case pb.CacheSharingOpt_LOCKED:
251
-		return e.getRefCacheDirNoCache(ctx, key, ref, id, m, true)
272
+		return g.getRefCacheDirNoCache(ctx, key, ref, id, true)
252 273
 	default:
253 274
 		return nil, errors.Errorf("invalid cache sharing option: %s", sharing.String())
254 275
 	}
255
-
256 276
 }
257 277
 
258
-func (e *execOp) getRefCacheDirNoCache(ctx context.Context, key string, ref cache.ImmutableRef, id string, m *pb.Mount, block bool) (cache.MutableRef, error) {
278
+func (g *cacheRefGetter) getRefCacheDirNoCache(ctx context.Context, key string, ref cache.ImmutableRef, id string, block bool) (cache.MutableRef, error) {
259 279
 	makeMutable := func(ref cache.ImmutableRef) (cache.MutableRef, error) {
260
-		desc := fmt.Sprintf("cached mount %s from exec %s", m.Dest, strings.Join(e.op.Meta.Args, " "))
261
-		return e.cm.New(ctx, ref, cache.WithRecordType(client.UsageRecordTypeCacheMount), cache.WithDescription(desc), cache.CachePolicyRetain)
280
+		return g.cm.New(ctx, ref, cache.WithRecordType(client.UsageRecordTypeCacheMount), cache.WithDescription(g.name), cache.CachePolicyRetain)
262 281
 	}
263 282
 
264 283
 	cacheRefsLocker.Lock(key)
265 284
 	defer cacheRefsLocker.Unlock(key)
266 285
 	for {
267
-		sis, err := e.md.Search(key)
286
+		sis, err := g.md.Search(key)
268 287
 		if err != nil {
269 288
 			return nil, err
270 289
 		}
271 290
 		locked := false
272 291
 		for _, si := range sis {
273
-			if mRef, err := e.cm.GetMutable(ctx, si.ID()); err == nil {
292
+			if mRef, err := g.cm.GetMutable(ctx, si.ID()); err == nil {
274 293
 				logrus.Debugf("reusing ref for cache dir: %s", mRef.ID())
275 294
 				return mRef, nil
276 295
 			} else if errors.Cause(err) == cache.ErrLocked {
... ...
@@ -295,7 +315,7 @@ func (e *execOp) getRefCacheDirNoCache(ctx context.Context, key string, ref cach
295 295
 		return nil, err
296 296
 	}
297 297
 
298
-	si, _ := e.md.Get(mRef.ID())
298
+	si, _ := g.md.Get(mRef.ID())
299 299
 	v, err := metadata.NewValue(key)
300 300
 	if err != nil {
301 301
 		mRef.Release(context.TODO())
... ...
@@ -797,6 +817,9 @@ func CacheMountsLocker() sync.Locker {
797 797
 }
798 798
 
799 799
 func (r *cacheRefs) get(key string, fn func() (cache.MutableRef, error)) (cache.MutableRef, error) {
800
+	r.mu.Lock()
801
+	defer r.mu.Unlock()
802
+
800 803
 	if r.shares == nil {
801 804
 		r.shares = map[string]*cacheRefShare{}
802 805
 	}
... ...
@@ -813,7 +836,6 @@ func (r *cacheRefs) get(key string, fn func() (cache.MutableRef, error)) (cache.
813 813
 
814 814
 	share = &cacheRefShare{MutableRef: mref, main: r, key: key, refs: map[*cacheRef]struct{}{}}
815 815
 	r.shares[key] = share
816
-
817 816
 	return share.clone(), nil
818 817
 }
819 818
 
... ...
@@ -827,6 +849,9 @@ type cacheRefShare struct {
827 827
 
828 828
 func (r *cacheRefShare) clone() cache.MutableRef {
829 829
 	cacheRef := &cacheRef{cacheRefShare: r}
830
+	if cacheRefCloneHijack != nil {
831
+		cacheRefCloneHijack()
832
+	}
830 833
 	r.mu.Lock()
831 834
 	r.refs[cacheRef] = struct{}{}
832 835
 	r.mu.Unlock()
... ...
@@ -835,22 +860,30 @@ func (r *cacheRefShare) clone() cache.MutableRef {
835 835
 
836 836
 func (r *cacheRefShare) release(ctx context.Context) error {
837 837
 	if r.main != nil {
838
-		r.main.mu.Lock()
839
-		defer r.main.mu.Unlock()
840 838
 		delete(r.main.shares, r.key)
841 839
 	}
842 840
 	return r.MutableRef.Release(ctx)
843 841
 }
844 842
 
843
+var cacheRefReleaseHijack func()
844
+var cacheRefCloneHijack func()
845
+
845 846
 type cacheRef struct {
846 847
 	*cacheRefShare
847 848
 }
848 849
 
849 850
 func (r *cacheRef) Release(ctx context.Context) error {
851
+	if r.main != nil {
852
+		r.main.mu.Lock()
853
+		defer r.main.mu.Unlock()
854
+	}
850 855
 	r.mu.Lock()
851 856
 	defer r.mu.Unlock()
852 857
 	delete(r.refs, r)
853 858
 	if len(r.refs) == 0 {
859
+		if cacheRefReleaseHijack != nil {
860
+			cacheRefReleaseHijack()
861
+		}
854 862
 		return r.release(ctx)
855 863
 	}
856 864
 	return nil