chunked: fix reuse of the layers cache

the global singleton was never updated, causing the cache to be always
recreated for each layer.

It is not possible to keep the layersCache mutex for the entire load()
since it calls into some store APIs causing a deadlock since
findDigestInternal() is already called while some store locks are
held.

Another benefit is that now only one goroutine can run load()
preventing multiple calls to load() to happen in parallel doing the
same work.

Closes: #2023

Signed-off-by: Giuseppe Scrivano <[email protected]>

layers.go

@@ -314,9 +314,9 @@ 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.
+	applyDiffWithDifferNoLock(layer *Layer, options *drivers.ApplyDiffWithDifferOpts, differ drivers.Differ) (*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 +2543,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 +2563,11 @@ 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
 	}
-	layer.UIDs = output.UIDs
-	layer.GIDs = output.GIDs
-	err = r.saveFor(layer)
-	return &output, err
+	return &out, nil
 }
 
 func (r *layerStore) CleanupStagingDirectory(stagingDirectory string) error {

pkg/chunked/cache_linux.go

@@ -82,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
 	}
 }
 
@@ -110,7 +111,7 @@ func getLayersCacheRef(store storage.Store) *layersCache {
 		cache.refs++
 		return cache
 	}
-	cache := &layersCache{
+	cache = &layersCache{
 		store: store,
 		refs:  1,
 	}
@@ -779,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

@@ -3114,12 +3114,16 @@ func (s *store) PrepareStagedLayer(options *drivers.ApplyDiffWithDifferOpts, dif
 }
 
 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
-		}
-		return rlstore.ApplyDiffWithDiffer(to, options, differ)
-	})
+	rlstore, err := s.getLayerStore()
+	if err != nil {
+		return nil, err
+	}
+
+	if to != "" {
+		return nil, fmt.Errorf("ApplyDiffWithDiffer does not support non-empty 'layer' parameter")
+	}
+
+	return rlstore.applyDiffWithDifferNoLock(nil, options, differ)
 }
 
 func (s *store) DifferTarget(id string) (string, error) {