chunked: fix reuse of the layers cache

layers.go

@@ -314,9 +314,15 @@ type rwLayerStore interface {
 	// applies its changes to a specified layer.
 	ApplyDiff(to string, diff io.Reader) (int64, error)
 
-	// ApplyDiffWithDiffer applies the changes through the differ callback function.
-	// If to is the empty string, then a staging directory is created by the driver.
-	ApplyDiffWithDiffer(to string, options *drivers.ApplyDiffWithDifferOpts, differ drivers.Differ) (*drivers.DriverWithDifferOutput, error)
+	// ApplyDiffWithDifferNoLock applies the changes through the differ callback function.
+	// If layer is nil, then a staging directory is created by the driver.
+	// It is the caller's responsibility to call ApplyDiffWithDifferFinalize to store the changes performed by this function.
+	ApplyDiffWithDifferNoLock(layer *Layer, options *drivers.ApplyDiffWithDifferOpts, differ drivers.Differ) (*drivers.DriverWithDifferOutput, error)
+
+	// ApplyDiffWithDifferFinalize stores the changes performed by ApplyDiffWithDifferNoLock.
+	// ApplyDiffWithDifferNoLock runs without holding the lock, so this function is used to finalize the changes while it has exclusive access
+	// to the store.
+	ApplyDiffWithDifferFinalize(id string, output *drivers.DriverWithDifferOutput) error
 
 	// CleanupStagingDirectory cleanups the staging directory.  It can be used to cleanup the staging directory on errors
 	CleanupStagingDirectory(stagingDirectory string) error
@@ -2543,22 +2549,18 @@ func (r *layerStore) applyDiffFromStagingDirectory(id string, diffOutput *driver
 	return err
 }
 
-// Requires startWriting.
-func (r *layerStore) ApplyDiffWithDiffer(to string, options *drivers.ApplyDiffWithDifferOpts, differ drivers.Differ) (*drivers.DriverWithDifferOutput, error) {
+// It must be called without any c/storage locks held to allow differ to make c/storage calls.
+func (r *layerStore) ApplyDiffWithDifferNoLock(layer *Layer, options *drivers.ApplyDiffWithDifferOpts, differ drivers.Differ) (*drivers.DriverWithDifferOutput, error) {
 	ddriver, ok := r.driver.(drivers.DriverWithDiffer)
 	if !ok {
 		return nil, ErrNotSupported
 	}
 
-	if to == "" {
+	if layer == nil {
 		output, err := ddriver.ApplyDiffWithDiffer("", "", options, differ)
 		return &output, err
 	}
 
-	layer, ok := r.lookup(to)
-	if !ok {
-		return nil, ErrLayerUnknown
-	}
 	if options == nil {
 		options = &drivers.ApplyDiffWithDifferOpts{
 			ApplyDiffOpts: drivers.ApplyDiffOpts{
@@ -2567,14 +2569,22 @@ func (r *layerStore) ApplyDiffWithDiffer(to string, options *drivers.ApplyDiffWi
 			},
 		}
 	}
-	output, err := ddriver.ApplyDiffWithDiffer(layer.ID, layer.Parent, options, differ)
+	out, err := ddriver.ApplyDiffWithDiffer(layer.ID, layer.Parent, options, differ)
 	if err != nil {
 		return nil, err
 	}
+	return &out, nil
+}
+
+// Requires startWriting, must be called after ApplyDiffWithDifferNoLock
+func (r *layerStore) ApplyDiffWithDifferFinalize(id string, output *drivers.DriverWithDifferOutput) error {
+	layer, ok := r.lookup(id)
+	if !ok {
+		return ErrLayerUnknown
+	}
 	layer.UIDs = output.UIDs
 	layer.GIDs = output.GIDs
-	err = r.saveFor(layer)
-	return &output, err
+	return r.saveFor(layer)
 }
 
 func (r *layerStore) CleanupStagingDirectory(stagingDirectory string) error {

pkg/chunked/cache_linux.go

@@ -69,7 +69,6 @@ type layersCache struct {
 	refs    int
 	store   storage.Store
 	mutex   sync.RWMutex
-	created time.Time
 }
 
 var (
@@ -83,6 +82,7 @@ func (c *layer) release() {
 		if err := unix.Munmap(c.mmapBuffer); err != nil {
 			logrus.Warnf("Error Munmap: layer %q: %v", c.id, err)
 		}
+		c.mmapBuffer = nil
 	}
 }
 
@@ -107,14 +107,13 @@ func (c *layersCache) release() {
 func getLayersCacheRef(store storage.Store) *layersCache {
 	cacheMutex.Lock()
 	defer cacheMutex.Unlock()
-	if cache != nil && cache.store == store && time.Since(cache.created).Minutes() < 10 {
+	if cache != nil && cache.store == store {
 		cache.refs++
 		return cache
 	}
-	cache := &layersCache{
+	cache = &layersCache{
 		store:   store,
 		refs:    1,
-		created: time.Now(),
 	}
 	return cache
 }
@@ -781,14 +780,14 @@ func (c *layersCache) findDigestInternal(digest string) (string, string, int64,
 		return "", "", -1, nil
 	}
 
+	c.mutex.RLock()
+	defer c.mutex.RUnlock()
+
 	binaryDigest, err := makeBinaryDigest(digest)
 	if err != nil {
 		return "", "", 0, err
 	}
 
-	c.mutex.RLock()
-	defer c.mutex.RUnlock()
-
 	for _, layer := range c.layers {
 		if !layer.cacheFile.bloomFilter.maybeContains(binaryDigest) {
 			continue

store.go

@@ -3104,12 +3104,41 @@ func (s *store) CleanupStagedLayer(diffOutput *drivers.DriverWithDifferOutput) e
 }
 
 func (s *store) ApplyDiffWithDiffer(to string, options *drivers.ApplyDiffWithDifferOpts, differ drivers.Differ) (*drivers.DriverWithDifferOutput, error) {
-	return writeToLayerStore(s, func(rlstore rwLayerStore) (*drivers.DriverWithDifferOutput, error) {
-		if to != "" && !rlstore.Exists(to) {
-			return nil, ErrLayerUnknown
+	rlstore, err := s.getLayerStore()
+	if err != nil {
+		return nil, err
+	}
+
+	var layer *Layer
+	if to != "" {
+		if err := rlstore.startReading(); err != nil {
+			return nil, err
 		}
-		return rlstore.ApplyDiffWithDiffer(to, options, differ)
-	})
+		layer, err = rlstore.Get(to)
+		rlstore.stopReading()
+		if err != nil {
+			return nil, err
+		}
+	}
+
+	out, err := rlstore.ApplyDiffWithDifferNoLock(layer, options, differ)
+	if err != nil {
+		return nil, err
+	}
+
+	if layer != nil {
+		if err := rlstore.startWriting(); err != nil {
+			_ = s.CleanupStagedLayer(out)
+			return nil, err
+		}
+		err = rlstore.ApplyDiffWithDifferFinalize(layer.ID, out)
+		rlstore.stopWriting()
+		if err != nil {
+			_ = s.CleanupStagedLayer(out)
+			return nil, err
+		}
+	}
+	return out, nil
 }
 
 func (s *store) DifferTarget(id string) (string, error) {