Remove the runtime lock

This primarily served to protect us against shutting down the
Libpod runtime while operations (like creating a container) were
happening. However, it was very inconsistently implemented (a lot
of our longer-lived functions, like pulling images, just didn't
implement it at all...) and I'm not sure how much we really care
about this very-specific error case?

Removing it also removes a lot of potential deadlocks, which is
nice.

[NO NEW TESTS NEEDED]

Signed-off-by: Matthew Heon <mheon@redhat.com>

libpod/networking_linux.go

@@ -320,7 +320,7 @@ func (r *RootlessNetNS) Cleanup(runtime *Runtime) error {
 		// only if the netns is empty we know that we do not need cleanup
 		return c.state.NetNS != nil
 	}
-	ctrs, err := runtime.GetContainersWithoutLock(activeNetns)
+	ctrs, err := runtime.GetContainers(activeNetns)
 	if err != nil {
 		return err
 	}

libpod/runtime.go

@@ -11,7 +11,6 @@ import (
 	"regexp"
 	"strconv"
 	"strings"
-	"sync"
 	"syscall"
 	"time"
 
@@ -109,7 +108,6 @@ type Runtime struct {
 	// and remains true until the runtime is shut down (rendering its
 	// storage unusable). When valid is false, the runtime cannot be used.
 	valid bool
-	lock  sync.RWMutex
 
 	// mechanism to read and write even logs
 	eventer events.Eventer
@@ -713,9 +711,6 @@ func (r *Runtime) TmpDir() (string, error) {
 // Note that the returned value is not a copy and must hence
 // only be used in a reading fashion.
 func (r *Runtime) GetConfigNoCopy() (*config.Config, error) {
-	r.lock.RLock()
-	defer r.lock.RUnlock()
-
 	if !r.valid {
 		return nil, define.ErrRuntimeStopped
 	}
@@ -810,9 +805,6 @@ func (r *Runtime) DeferredShutdown(force bool) {
 // cleaning up; if force is false, an error will be returned if there are
 // still containers running or mounted
 func (r *Runtime) Shutdown(force bool) error {
-	r.lock.Lock()
-	defer r.lock.Unlock()
-
 	if !r.valid {
 		return define.ErrRuntimeStopped
 	}
@@ -1016,9 +1008,6 @@ func (r *Runtime) RunRoot() string {
 // If the given ID does not correspond to any existing Pod or Container,
 // ErrNoSuchCtr is returned.
 func (r *Runtime) GetName(id string) (string, error) {
-	r.lock.RLock()
-	defer r.lock.RUnlock()
-
 	if !r.valid {
 		return "", define.ErrRuntimeStopped
 	}

libpod/runtime_cstorage.go

@@ -21,9 +21,6 @@ type StorageContainer struct {
 
 // ListStorageContainers lists all containers visible to c/storage.
 func (r *Runtime) ListStorageContainers() ([]*StorageContainer, error) {
-	r.lock.RLock()
-	defer r.lock.RUnlock()
-
 	finalCtrs := []*StorageContainer{}
 
 	ctrs, err := r.store.Containers()
@@ -61,15 +58,6 @@ func (r *Runtime) StorageContainer(idOrName string) (*storage.Container, error)
 // Accepts ID or full name of container.
 // If force is set, the container will be unmounted first to ensure removal.
 func (r *Runtime) RemoveStorageContainer(idOrName string, force bool) error {
-	r.lock.Lock()
-	defer r.lock.Unlock()
-
-	return r.removeStorageContainer(idOrName, force)
-}
-
-// Internal function to remove the container storage without
-// locking the runtime.
-func (r *Runtime) removeStorageContainer(idOrName string, force bool) error {
 	targetID, err := r.store.Lookup(idOrName)
 	if err != nil {
 		if errors.Cause(err) == storage.ErrLayerUnknown {

libpod/runtime_ctr.go

@@ -42,8 +42,6 @@ type ContainerFilter func(*Container) bool
 
 // NewContainer creates a new container from a given OCI config.
 func (r *Runtime) NewContainer(ctx context.Context, rSpec *spec.Spec, spec *specgen.SpecGenerator, infra bool, options ...CtrCreateOption) (*Container, error) {
-	r.lock.Lock()
-	defer r.lock.Unlock()
 	if !r.valid {
 		return nil, define.ErrRuntimeStopped
 	}
@@ -81,8 +79,6 @@ func (r *Runtime) PrepareVolumeOnCreateContainer(ctx context.Context, ctr *Conta
 
 // RestoreContainer re-creates a container from an imported checkpoint
 func (r *Runtime) RestoreContainer(ctx context.Context, rSpec *spec.Spec, config *ContainerConfig) (*Container, error) {
-	r.lock.Lock()
-	defer r.lock.Unlock()
 	if !r.valid {
 		return nil, define.ErrRuntimeStopped
 	}
@@ -545,8 +541,6 @@ func (r *Runtime) setupContainer(ctx context.Context, ctr *Container) (_ *Contai
 // be removed also if and only if the container is the sole user
 // Otherwise, RemoveContainer will return an error if the container is running
 func (r *Runtime) RemoveContainer(ctx context.Context, c *Container, force bool, removeVolume bool, timeout *uint) error {
-	r.lock.Lock()
-	defer r.lock.Unlock()
 	return r.removeContainer(ctx, c, force, removeVolume, false, timeout)
 }
 
@@ -784,8 +778,6 @@ func (r *Runtime) removeContainer(ctx context.Context, c *Container, force, remo
 // If removeVolume is specified, named volumes used by the container will
 // be removed also if and only if the container is the sole user.
 func (r *Runtime) EvictContainer(ctx context.Context, idOrName string, removeVolume bool) (string, error) {
-	r.lock.RLock()
-	defer r.lock.RUnlock()
 	return r.evictContainer(ctx, idOrName, removeVolume)
 }
 
@@ -894,7 +886,7 @@ func (r *Runtime) evictContainer(ctx context.Context, idOrName string, removeVol
 	}
 
 	// Remove container from c/storage
-	if err := r.removeStorageContainer(id, true); err != nil {
+	if err := r.RemoveStorageContainer(id, true); err != nil {
 		if cleanupErr == nil {
 			cleanupErr = err
 		}
@@ -972,9 +964,6 @@ func (r *Runtime) RemoveDepend(ctx context.Context, rmCtr *Container, force bool
 
 // GetContainer retrieves a container by its ID
 func (r *Runtime) GetContainer(id string) (*Container, error) {
-	r.lock.RLock()
-	defer r.lock.RUnlock()
-
 	if !r.valid {
 		return nil, define.ErrRuntimeStopped
 	}
@@ -984,9 +973,6 @@ func (r *Runtime) GetContainer(id string) (*Container, error) {
 
 // HasContainer checks if a container with the given ID is present
 func (r *Runtime) HasContainer(id string) (bool, error) {
-	r.lock.RLock()
-	defer r.lock.RUnlock()
-
 	if !r.valid {
 		return false, define.ErrRuntimeStopped
 	}
@@ -997,9 +983,6 @@ func (r *Runtime) HasContainer(id string) (bool, error) {
 // LookupContainer looks up a container by its name or a partial ID
 // If a partial ID is not unique, an error will be returned
 func (r *Runtime) LookupContainer(idOrName string) (*Container, error) {
-	r.lock.RLock()
-	defer r.lock.RUnlock()
-
 	if !r.valid {
 		return nil, define.ErrRuntimeStopped
 	}
@@ -1009,9 +992,6 @@ func (r *Runtime) LookupContainer(idOrName string) (*Container, error) {
 // LookupContainerId looks up a container id by its name or a partial ID
 // If a partial ID is not unique, an error will be returned
 func (r *Runtime) LookupContainerID(idOrName string) (string, error) {
-	r.lock.RLock()
-	defer r.lock.RUnlock()
-
 	if !r.valid {
 		return "", define.ErrRuntimeStopped
 	}
@@ -1023,13 +1003,6 @@ func (r *Runtime) LookupContainerID(idOrName string) (string, error) {
 // the output. Multiple filters are handled by ANDing their output, so only
 // containers matching all filters are returned
 func (r *Runtime) GetContainers(filters ...ContainerFilter) ([]*Container, error) {
-	r.lock.RLock()
-	defer r.lock.RUnlock()
-	return r.GetContainersWithoutLock(filters...)
-}
-
-// GetContainersWithoutLock is same as GetContainers but without lock
-func (r *Runtime) GetContainersWithoutLock(filters ...ContainerFilter) ([]*Container, error) {
 	if !r.valid {
 		return nil, define.ErrRuntimeStopped
 	}
@@ -1107,9 +1080,6 @@ func (r *Runtime) GetLatestContainer() (*Container, error) {
 // GetExecSessionContainer gets the container that a given exec session ID is
 // attached to.
 func (r *Runtime) GetExecSessionContainer(id string) (*Container, error) {
-	r.lock.RLock()
-	defer r.lock.RUnlock()
-
 	if !r.valid {
 		return nil, define.ErrRuntimeStopped
 	}

libpod/runtime_img.go

@@ -25,9 +25,6 @@ import (
 // we can use the libpod-internal removal logic.
 func (r *Runtime) RemoveContainersForImageCallback(ctx context.Context) libimage.RemoveContainerFunc {
 	return func(imageID string) error {
-		r.lock.Lock()
-		defer r.lock.Unlock()
-
 		if !r.valid {
 			return define.ErrRuntimeStopped
 		}

libpod/runtime_pod.go

@@ -27,9 +27,6 @@ type PodFilter func(*Pod) bool
 // being removed
 // Otherwise, the pod will not be removed if any containers are running
 func (r *Runtime) RemovePod(ctx context.Context, p *Pod, removeCtrs, force bool, timeout *uint) error {
-	r.lock.Lock()
-	defer r.lock.Unlock()
-
 	if !r.valid {
 		return define.ErrRuntimeStopped
 	}
@@ -50,9 +47,6 @@ func (r *Runtime) RemovePod(ctx context.Context, p *Pod, removeCtrs, force bool,
 
 // GetPod retrieves a pod by its ID
 func (r *Runtime) GetPod(id string) (*Pod, error) {
-	r.lock.RLock()
-	defer r.lock.RUnlock()
-
 	if !r.valid {
 		return nil, define.ErrRuntimeStopped
 	}
@@ -62,9 +56,6 @@ func (r *Runtime) GetPod(id string) (*Pod, error) {
 
 // HasPod checks to see if a pod with the given ID exists
 func (r *Runtime) HasPod(id string) (bool, error) {
-	r.lock.RLock()
-	defer r.lock.RUnlock()
-
 	if !r.valid {
 		return false, define.ErrRuntimeStopped
 	}
@@ -75,9 +66,6 @@ func (r *Runtime) HasPod(id string) (bool, error) {
 // LookupPod retrieves a pod by its name or a partial ID
 // If a partial ID is not unique, an error will be returned
 func (r *Runtime) LookupPod(idOrName string) (*Pod, error) {
-	r.lock.RLock()
-	defer r.lock.RUnlock()
-
 	if !r.valid {
 		return nil, define.ErrRuntimeStopped
 	}
@@ -111,9 +99,6 @@ func (r *Runtime) Pods(filters ...PodFilter) ([]*Pod, error) {
 
 // GetAllPods retrieves all pods
 func (r *Runtime) GetAllPods() ([]*Pod, error) {
-	r.lock.RLock()
-	defer r.lock.RUnlock()
-
 	if !r.valid {
 		return nil, define.ErrRuntimeStopped
 	}
@@ -148,9 +133,6 @@ func (r *Runtime) GetRunningPods() ([]*Pod, error) {
 		pods        []string
 		runningPods []*Pod
 	)
-	r.lock.RLock()
-	defer r.lock.RUnlock()
-
 	if !r.valid {
 		return nil, define.ErrRuntimeStopped
 	}

libpod/runtime_pod_linux.go

@@ -22,9 +22,6 @@ import (
 
 // NewPod makes a new, empty pod
 func (r *Runtime) NewPod(ctx context.Context, p specgen.PodSpecGenerator, options ...PodCreateOption) (_ *Pod, deferredErr error) {
-	r.lock.Lock()
-	defer r.lock.Unlock()
-
 	if !r.valid {
 		return nil, define.ErrRuntimeStopped
 	}
@@ -151,9 +148,6 @@ func (r *Runtime) NewPod(ctx context.Context, p specgen.PodSpecGenerator, option
 
 // AddInfra adds the created infra container to the pod state
 func (r *Runtime) AddInfra(ctx context.Context, pod *Pod, infraCtr *Container) (*Pod, error) {
-	r.lock.Lock()
-	defer r.lock.Unlock()
-
 	if !r.valid {
 		return nil, define.ErrRuntimeStopped
 	}
@@ -167,9 +161,6 @@ func (r *Runtime) AddInfra(ctx context.Context, pod *Pod, infraCtr *Container) (
 
 // SavePod is a helper function to save the pod state from outside of libpod
 func (r *Runtime) SavePod(pod *Pod) error {
-	r.lock.Lock()
-	defer r.lock.Unlock()
-
 	if !r.valid {
 		return define.ErrRuntimeStopped
 	}

libpod/runtime_volume.go

@@ -22,9 +22,6 @@ type VolumeFilter func(*Volume) bool
 
 // RemoveVolume removes a volumes
 func (r *Runtime) RemoveVolume(ctx context.Context, v *Volume, force bool, timeout *uint) error {
-	r.lock.Lock()
-	defer r.lock.Unlock()
-
 	if !r.valid {
 		return define.ErrRuntimeStopped
 	}
@@ -41,9 +38,6 @@ func (r *Runtime) RemoveVolume(ctx context.Context, v *Volume, force bool, timeo
 
 // GetVolume retrieves a volume given its full name.
 func (r *Runtime) GetVolume(name string) (*Volume, error) {
-	r.lock.RLock()
-	defer r.lock.RUnlock()
-
 	if !r.valid {
 		return nil, define.ErrRuntimeStopped
 	}
@@ -58,9 +52,6 @@ func (r *Runtime) GetVolume(name string) (*Volume, error) {
 
 // LookupVolume retrieves a volume by unambiguous partial name.
 func (r *Runtime) LookupVolume(name string) (*Volume, error) {
-	r.lock.RLock()
-	defer r.lock.RUnlock()
-
 	if !r.valid {
 		return nil, define.ErrRuntimeStopped
 	}
@@ -75,9 +66,6 @@ func (r *Runtime) LookupVolume(name string) (*Volume, error) {
 
 // HasVolume checks to see if a volume with the given name exists
 func (r *Runtime) HasVolume(name string) (bool, error) {
-	r.lock.RLock()
-	defer r.lock.RUnlock()
-
 	if !r.valid {
 		return false, define.ErrRuntimeStopped
 	}
@@ -90,9 +78,6 @@ func (r *Runtime) HasVolume(name string) (bool, error) {
 // output. If multiple filters are used, a volume will be returned if
 // any of the filters are matched
 func (r *Runtime) Volumes(filters ...VolumeFilter) ([]*Volume, error) {
-	r.lock.RLock()
-	defer r.lock.RUnlock()
-
 	if !r.valid {
 		return nil, define.ErrRuntimeStopped
 	}
@@ -123,9 +108,6 @@ func (r *Runtime) Volumes(filters ...VolumeFilter) ([]*Volume, error) {
 
 // GetAllVolumes retrieves all the volumes
 func (r *Runtime) GetAllVolumes() ([]*Volume, error) {
-	r.lock.RLock()
-	defer r.lock.RUnlock()
-
 	if !r.valid {
 		return nil, define.ErrRuntimeStopped
 	}

libpod/runtime_volume_linux.go

@@ -21,9 +21,6 @@ import (
 
 // NewVolume creates a new empty volume
 func (r *Runtime) NewVolume(ctx context.Context, options ...VolumeCreateOption) (*Volume, error) {
-	r.lock.Lock()
-	defer r.lock.Unlock()
-
 	if !r.valid {
 		return nil, define.ErrRuntimeStopped
 	}