Replace containerStore.{RLock,Unlock} with {startReading,stopReading}

This integrates ReloadIfChanges, and makes it clearer that the responsibility
for maintaining locking details is with the containerStore; we can change it
in a single place.

Should not change behavior.

Signed-off-by: Miloslav Trmač <mitr@redhat.com>

containers.go

@@ -84,8 +84,12 @@ type rwContainerStore interface {
 	// - if the lock counter is corrupted
 	Unlock()
 
-	// Acquire a reader lock.
+	// startReading makes sure the store is fresh, and locks it for reading.
-	RLock()
+	// If this succeeds, the caller MUST call stopReading().
+	startReading() error
+
+	// stopReading releases locks obtained by startReading.
+	stopReading()
 
 	// Touch records, for others sharing the lock, that the caller was the
 	// last writer.  It should only be called with the lock held.
@@ -205,6 +209,30 @@ func (c *Container) MountOpts() []string {
 	}
 }
 
+// startReading makes sure the store is fresh, and locks it for reading.
+// If this succeeds, the caller MUST call stopReading().
+func (r *containerStore) startReading() error {
+	r.lockfile.RLock()
+	succeeded := false
+	defer func() {
+		if !succeeded {
+			r.lockfile.Unlock()
+		}
+	}()
+
+	if err := r.ReloadIfChanged(); err != nil {
+		return err
+	}
+
+	succeeded = true
+	return nil
+}
+
+// stopReading releases locks obtained by startReading.
+func (r *containerStore) stopReading() {
+	r.lockfile.Unlock()
+}
+
 func (r *containerStore) Containers() ([]Container, error) {
 	containers := make([]Container, len(r.containers))
 	for i := range r.containers {
@@ -642,10 +670,6 @@ func (r *containerStore) Lock() {
 	r.lockfile.Lock()
 }
 
-func (r *containerStore) RLock() {
-	r.lockfile.RLock()
-}
-
 func (r *containerStore) Unlock() {
 	r.lockfile.Unlock()
 }

store.go

@@ -1664,11 +1664,10 @@ func (s *store) Metadata(id string) (string, error) {
 	if err != nil {
 		return "", err
 	}
-	cstore.RLock()
+	if err := cstore.startReading(); err != nil {
-	defer cstore.Unlock()
-	if err := cstore.ReloadIfChanged(); err != nil {
 		return "", err
 	}
+	defer cstore.stopReading()
 	if cstore.Exists(id) {
 		return cstore.Metadata(id)
 	}
@@ -1933,11 +1932,10 @@ func (s *store) ContainerSize(id string) (int64, error) {
 	if err != nil {
 		return -1, err
 	}
-	rcstore.RLock()
+	if err := rcstore.startReading(); err != nil {
-	defer rcstore.Unlock()
-	if err := rcstore.ReloadIfChanged(); err != nil {
 		return -1, err
 	}
+	defer rcstore.stopReading()
 
 	// Read the container record.
 	container, err := rcstore.Get(id)
@@ -1995,11 +1993,10 @@ func (s *store) ListContainerBigData(id string) ([]string, error) {
 		return nil, err
 	}
 
-	rcstore.RLock()
+	if err := rcstore.startReading(); err != nil {
-	defer rcstore.Unlock()
-	if err := rcstore.ReloadIfChanged(); err != nil {
 		return nil, err
 	}
+	defer rcstore.stopReading()
 
 	return rcstore.BigDataNames(id)
 }
@@ -2009,11 +2006,10 @@ func (s *store) ContainerBigDataSize(id, key string) (int64, error) {
 	if err != nil {
 		return -1, err
 	}
-	rcstore.RLock()
+	if err := rcstore.startReading(); err != nil {
-	defer rcstore.Unlock()
-	if err := rcstore.ReloadIfChanged(); err != nil {
 		return -1, err
 	}
+	defer rcstore.stopReading()
 	return rcstore.BigDataSize(id, key)
 }
 
@@ -2022,11 +2018,10 @@ func (s *store) ContainerBigDataDigest(id, key string) (digest.Digest, error) {
 	if err != nil {
 		return "", err
 	}
-	rcstore.RLock()
+	if err := rcstore.startReading(); err != nil {
-	defer rcstore.Unlock()
-	if err := rcstore.ReloadIfChanged(); err != nil {
 		return "", err
 	}
+	defer rcstore.stopReading()
 	return rcstore.BigDataDigest(id, key)
 }
 
@@ -2035,11 +2030,10 @@ func (s *store) ContainerBigData(id, key string) ([]byte, error) {
 	if err != nil {
 		return nil, err
 	}
-	rcstore.RLock()
+	if err := rcstore.startReading(); err != nil {
-	defer rcstore.Unlock()
-	if err := rcstore.ReloadIfChanged(); err != nil {
 		return nil, err
 	}
+	defer rcstore.stopReading()
 	return rcstore.BigData(id, key)
 }
 
@@ -2076,11 +2070,10 @@ func (s *store) Exists(id string) bool {
 	if err != nil {
 		return false
 	}
-	rcstore.RLock()
+	if err := rcstore.startReading(); err != nil {
-	defer rcstore.Unlock()
-	if err := rcstore.ReloadIfChanged(); err != nil {
 		return false
 	}
+	defer rcstore.stopReading()
 	if rcstore.Exists(id) {
 		return true
 	}
@@ -2206,11 +2199,10 @@ func (s *store) Names(id string) ([]string, error) {
 	if err != nil {
 		return nil, err
 	}
-	rcstore.RLock()
+	if err := rcstore.startReading(); err != nil {
-	defer rcstore.Unlock()
-	if err := rcstore.ReloadIfChanged(); err != nil {
 		return nil, err
 	}
+	defer rcstore.stopReading()
 	if c, err := rcstore.Get(id); c != nil && err == nil {
 		return c.Names, nil
 	}
@@ -2244,11 +2236,10 @@ func (s *store) Lookup(name string) (string, error) {
 	if err != nil {
 		return "", err
 	}
-	cstore.RLock()
+	if err := cstore.startReading(); err != nil {
-	defer cstore.Unlock()
-	if err := cstore.ReloadIfChanged(); err != nil {
 		return "", err
 	}
+	defer cstore.stopReading()
 	if c, err := cstore.Get(name); c != nil && err == nil {
 		return c.ID, nil
 	}
@@ -2888,11 +2879,10 @@ func (s *store) ContainerParentOwners(id string) ([]int, []int, error) {
 	if err := rlstore.ReloadIfChanged(); err != nil {
 		return nil, nil, err
 	}
-	rcstore.RLock()
+	if err := rcstore.startReading(); err != nil {
-	defer rcstore.Unlock()
-	if err := rcstore.ReloadIfChanged(); err != nil {
 		return nil, nil, err
 	}
+	defer rcstore.stopReading()
 	container, err := rcstore.Get(id)
 	if err != nil {
 		return nil, nil, err
@@ -2939,11 +2929,10 @@ func (s *store) Containers() ([]Container, error) {
 		return nil, err
 	}
 
-	rcstore.RLock()
+	if err := rcstore.startReading(); err != nil {
-	defer rcstore.Unlock()
-	if err := rcstore.ReloadIfChanged(); err != nil {
 		return nil, err
 	}
+	defer rcstore.stopReading()
 
 	return rcstore.Containers()
 }
@@ -3104,11 +3093,10 @@ func (s *store) Container(id string) (*Container, error) {
 	if err != nil {
 		return nil, err
 	}
-	rcstore.RLock()
+	if err := rcstore.startReading(); err != nil {
-	defer rcstore.Unlock()
-	if err := rcstore.ReloadIfChanged(); err != nil {
 		return nil, err
 	}
+	defer rcstore.stopReading()
 
 	return rcstore.Get(id)
 }
@@ -3118,11 +3106,10 @@ func (s *store) ContainerLayerID(id string) (string, error) {
 	if err != nil {
 		return "", err
 	}
-	rcstore.RLock()
+	if err := rcstore.startReading(); err != nil {
-	defer rcstore.Unlock()
-	if err := rcstore.ReloadIfChanged(); err != nil {
 		return "", err
 	}
+	defer rcstore.stopReading()
 	container, err := rcstore.Get(id)
 	if err != nil {
 		return "", err
@@ -3139,11 +3126,10 @@ func (s *store) ContainerByLayer(id string) (*Container, error) {
 	if err != nil {
 		return nil, err
 	}
-	rcstore.RLock()
+	if err := rcstore.startReading(); err != nil {
-	defer rcstore.Unlock()
-	if err := rcstore.ReloadIfChanged(); err != nil {
 		return nil, err
 	}
+	defer rcstore.stopReading()
 	containerList, err := rcstore.Containers()
 	if err != nil {
 		return nil, err
@@ -3162,11 +3148,10 @@ func (s *store) ContainerDirectory(id string) (string, error) {
 	if err != nil {
 		return "", err
 	}
-	rcstore.RLock()
+	if err := rcstore.startReading(); err != nil {
-	defer rcstore.Unlock()
-	if err := rcstore.ReloadIfChanged(); err != nil {
 		return "", err
 	}
+	defer rcstore.stopReading()
 
 	id, err = rcstore.Lookup(id)
 	if err != nil {
@@ -3187,11 +3172,10 @@ func (s *store) ContainerRunDirectory(id string) (string, error) {
 		return "", err
 	}
 
-	rcstore.RLock()
+	if err := rcstore.startReading(); err != nil {
-	defer rcstore.Unlock()
-	if err := rcstore.ReloadIfChanged(); err != nil {
 		return "", err
 	}
+	defer rcstore.stopReading()
 
 	id, err = rcstore.Lookup(id)
 	if err != nil {