From 203fcd69976fd36b06a59210eb15d5574b897aa3 Mon Sep 17 00:00:00 2001
From: Sebastiaan van Stijn <github@gone.nl>
Date: Mon, 28 Feb 2022 19:57:37 +0100
Subject: [PATCH] layers: remove layerStore.getWithoutLock()

This function was abstracting things a bit too much; the layerStore had a
exported `.Get()` which called `.getWithoutLock()`, but also a non-exported
`.get()`, which also called `.getWithoutLock()`.

While it's common to have a non-exported variant (without locking), the naming
of `.get()` could easily be confused for that variant (which it wasn't).

All locations where `.get()` was called were already handling locks for
`releaseLayer()`, so moving the actual locking inline for `.get()` makes it
more visible where locking happens.

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
---
 layer/layer_store.go | 18 +++++++-----------
 layer/migration.go   |  4 +++-
 2 files changed, 10 insertions(+), 12 deletions(-)

diff --git a/layer/layer_store.go b/layer/layer_store.go
index fbdd074a4d..7b4c26585b 100644
--- a/layer/layer_store.go
+++ b/layer/layer_store.go
@@ -284,7 +284,9 @@ func (ls *layerStore) registerWithDescriptor(ts io.Reader, parent ChainID, descr
 	var p *roLayer
 
 	if string(parent) != "" {
+		ls.layerL.Lock()
 		p = ls.get(parent)
+		ls.layerL.Unlock()
 		if p == nil {
 			return nil, ErrLayerDoesNotExist
 		}
@@ -351,7 +353,7 @@ func (ls *layerStore) registerWithDescriptor(ts io.Reader, parent ChainID, descr
 	ls.layerL.Lock()
 	defer ls.layerL.Unlock()
 
-	if existingLayer := ls.getWithoutLock(layer.chainID); existingLayer != nil {
+	if existingLayer := ls.get(layer.chainID); existingLayer != nil {
 		// Set error for cleanup, but do not return the error
 		err = errors.New("layer already exists")
 		return existingLayer.getReference(), nil
@@ -366,28 +368,20 @@ func (ls *layerStore) registerWithDescriptor(ts io.Reader, parent ChainID, descr
 	return layer.getReference(), nil
 }
 
-func (ls *layerStore) getWithoutLock(layer ChainID) *roLayer {
+func (ls *layerStore) get(layer ChainID) *roLayer {
 	l, ok := ls.layerMap[layer]
 	if !ok {
 		return nil
 	}
-
 	l.referenceCount++
-
 	return l
 }
 
-func (ls *layerStore) get(l ChainID) *roLayer {
-	ls.layerL.Lock()
-	defer ls.layerL.Unlock()
-	return ls.getWithoutLock(l)
-}
-
 func (ls *layerStore) Get(l ChainID) (Layer, error) {
 	ls.layerL.Lock()
 	defer ls.layerL.Unlock()
 
-	layer := ls.getWithoutLock(l)
+	layer := ls.get(l)
 	if layer == nil {
 		return nil, ErrLayerDoesNotExist
 	}
@@ -520,7 +514,9 @@ func (ls *layerStore) CreateRWLayer(name string, parent ChainID, opts *CreateRWL
 	var pid string
 	var p *roLayer
 	if string(parent) != "" {
+		ls.layerL.Lock()
 		p = ls.get(parent)
+		ls.layerL.Unlock()
 		if p == nil {
 			return nil, ErrLayerDoesNotExist
 		}
diff --git a/layer/migration.go b/layer/migration.go
index 80f0ff7ff4..5c8179a434 100644
--- a/layer/migration.go
+++ b/layer/migration.go
@@ -87,7 +87,9 @@ func (ls *layerStore) RegisterByGraphID(graphID string, parent ChainID, diffID D
 	var err error
 	var p *roLayer
 	if string(parent) != "" {
+		ls.layerL.Lock()
 		p = ls.get(parent)
+		ls.layerL.Unlock()
 		if p == nil {
 			return nil, ErrLayerDoesNotExist
 		}
@@ -117,7 +119,7 @@ func (ls *layerStore) RegisterByGraphID(graphID string, parent ChainID, diffID D
 	ls.layerL.Lock()
 	defer ls.layerL.Unlock()
 
-	if existingLayer := ls.getWithoutLock(layer.chainID); existingLayer != nil {
+	if existingLayer := ls.get(layer.chainID); existingLayer != nil {
 		// Set error for cleanup, but do not return
 		err = errors.New("layer already exists")
 		return existingLayer.getReference(), nil