Merge pull request #493 from saschagrunert/layer-store-load-lock

Remove locking from layer store creation

layers.go

@@ -239,6 +239,10 @@ type LayerStore interface {
 	// ApplyDiff reads a tarstream which was created by a previous call to Diff and
 	// applies its changes to a specified layer.
 	ApplyDiff(to string, diff io.Reader) (int64, error)
+
+	// LoadLocked wraps Load in a locked state. This means it loads the store
+	// and cleans-up invalid layers if needed.
+	LoadLocked() error
 }
 
 type layerStore struct {
@@ -346,39 +350,48 @@ func (r *layerStore) Load() error {
 	r.byname = names
 	r.bycompressedsum = compressedsums
 	r.byuncompressedsum = uncompressedsums
+
 	// Load and merge information about which layers are mounted, and where.
 	if r.IsReadWrite() {
 		r.mountsLockfile.RLock()
 		defer r.mountsLockfile.Unlock()
 		if err = r.loadMounts(); err != nil {
 			return err
 		}
-	}
-	// Last step: if we're writable, try to remove anything that a previous
-	// user of this storage area marked for deletion but didn't manage to
-	// actually delete.
-	if r.IsReadWrite() && r.Locked() {
-		for _, layer := range r.layers {
-			if layer.Flags == nil {
-				layer.Flags = make(map[string]interface{})
-			}
-			if cleanup, ok := layer.Flags[incompleteFlag]; ok {
-				if b, ok := cleanup.(bool); ok && b {
-					err = r.deleteInternal(layer.ID)
-					if err != nil {
-						break
+
+		// Last step: as we’re writable, try to remove anything that a previous
+		// user of this storage area marked for deletion but didn't manage to
+		// actually delete.
+		if r.Locked() {
+			for _, layer := range r.layers {
+				if layer.Flags == nil {
+					layer.Flags = make(map[string]interface{})
+				}
+				if cleanup, ok := layer.Flags[incompleteFlag]; ok {
+					if b, ok := cleanup.(bool); ok && b {
+						err = r.deleteInternal(layer.ID)
+						if err != nil {
+							break
+						}
+						shouldSave = true
 					}
-					shouldSave = true
 				}
 			}
 		}
 		if shouldSave {
 			return r.saveLayers()
 		}
 	}
+
 	return err
 }
 
+func (r *layerStore) LoadLocked() error {
+	r.lockfile.Lock()
+	defer r.lockfile.Unlock()
+	return r.Load()
+}
+
 func (r *layerStore) loadMounts() error {
 	mounts := make(map[string]*Layer)
 	mpath := r.mountspath()
@@ -487,8 +500,6 @@ func (s *store) newLayerStore(rundir string, layerdir string, driver drivers.Dri
 	if err != nil {
 		return nil, err
 	}
-	lockfile.Lock()
-	defer lockfile.Unlock()
 	mountsLockfile, err := GetLockfile(filepath.Join(rundir, "mountpoints.lock"))
 	if err != nil {
 		return nil, err
@@ -516,8 +527,6 @@ func newROLayerStore(rundir string, layerdir string, driver drivers.Driver) (ROL
 	if err != nil {
 		return nil, err
 	}
-	lockfile.RLock()
-	defer lockfile.Unlock()
 	rlstore := layerStore{
 		lockfile:       lockfile,
 		mountsLockfile: nil,

store.go

@@ -2783,18 +2783,24 @@ func (s *store) ContainerParentOwners(id string) ([]int, []int, error) {
 }
 
 func (s *store) Layers() ([]Layer, error) {
-	var layers []Layer
 	lstore, err := s.LayerStore()
 	if err != nil {
 		return nil, err
 	}
+	if err := lstore.LoadLocked(); err != nil {
+		return nil, err
+	}
+	layers, err := lstore.Layers()
+	if err != nil {
+		return nil, err
+	}
 
 	lstores, err := s.ROLayerStores()
 	if err != nil {
 		return nil, err
 	}
 
-	for _, s := range append([]ROLayerStore{lstore}, lstores...) {
+	for _, s := range lstores {
 		store := s
 		store.RLock()
 		defer store.Unlock()