diff --git a/libpod/container_api.go b/libpod/container_api.go index 7c95fb9fee..0a84a5e5b2 100644 --- a/libpod/container_api.go +++ b/libpod/container_api.go @@ -85,34 +85,23 @@ func (c *Container) initUnlocked(ctx context.Context, recursive bool) error { // Start requires that all dependency containers (e.g. pod infra containers) are // running before starting the container. The recursive parameter, if set, will start all // dependencies before starting this container. -func (c *Container) Start(ctx context.Context, recursive bool) (finalErr error) { - if !c.batched { - c.lock.Lock() - defer c.lock.Unlock() - - // defer's are executed LIFO so we are locked here - // as long as we call this after the defer unlock() - defer func() { - if finalErr != nil { - if err := saveContainerError(c, finalErr); err != nil { - logrus.Debug(err) - } - } - }() - - if err := c.syncContainer(); err != nil { - return err +func (c *Container) Start(ctx context.Context, recursive bool) error { + // Have to lock the pod the container is a part of. + // This prevents running `podman start` at the same time a + // `podman pod stop` is running, which could lead to wierd races. + // Pod locks come before container locks, so do this first. + if c.config.Pod != "" { + // If we get an error, the pod was probably removed. + // So we get an expected ErrCtrRemoved instead of ErrPodRemoved, + // just ignore this and move on to syncing the container. + pod, _ := c.runtime.state.Pod(c.config.Pod) + if pod != nil { + pod.lock.Lock() + defer pod.lock.Unlock() } } - if err := c.prepareToStart(ctx, recursive); err != nil { - return err - } - // Start the container - if err := c.start(); err != nil { - return err - } - return c.waitForHealthy(ctx) + return c.startNoPodLock(ctx, recursive) } // Update updates the given container. @@ -294,6 +283,21 @@ func (c *Container) Stop() error { // manually. If timeout is 0, SIGKILL will be used immediately to kill the // container. func (c *Container) StopWithTimeout(timeout uint) (finalErr error) { + // Have to lock the pod the container is a part of. + // This prevents running `podman stop` at the same time a + // `podman pod start` is running, which could lead to wierd races. + // Pod locks come before container locks, so do this first. + if c.config.Pod != "" { + // If we get an error, the pod was probably removed. + // So we get an expected ErrCtrRemoved instead of ErrPodRemoved, + // just ignore this and move on to syncing the container. + pod, _ := c.runtime.state.Pod(c.config.Pod) + if pod != nil { + pod.lock.Lock() + defer pod.lock.Unlock() + } + } + if !c.batched { c.lock.Lock() defer c.lock.Unlock() diff --git a/libpod/container_internal.go b/libpod/container_internal.go index 721d78d5ba..07f5c8d096 100644 --- a/libpod/container_internal.go +++ b/libpod/container_internal.go @@ -1260,6 +1260,40 @@ func (c *Container) initAndStart(ctx context.Context) (retErr error) { return c.waitForHealthy(ctx) } +// Internal function to start a container without taking the pod lock. +// Please note that this DOES take the container lock. +// Intended to be used in pod-related functions. +func (c *Container) startNoPodLock(ctx context.Context, recursive bool) (finalErr error) { + if !c.batched { + c.lock.Lock() + defer c.lock.Unlock() + + // defer's are executed LIFO so we are locked here + // as long as we call this after the defer unlock() + defer func() { + if finalErr != nil { + if err := saveContainerError(c, finalErr); err != nil { + logrus.Debug(err) + } + } + }() + + if err := c.syncContainer(); err != nil { + return err + } + } + + if err := c.prepareToStart(ctx, recursive); err != nil { + return err + } + + // Start the container + if err := c.start(); err != nil { + return err + } + return c.waitForHealthy(ctx) +} + // Internal, non-locking function to start a container func (c *Container) start() error { if c.config.Spec.Process != nil { diff --git a/libpod/pod_api.go b/libpod/pod_api.go index 10ac424b8d..353c50f40f 100644 --- a/libpod/pod_api.go +++ b/libpod/pod_api.go @@ -24,7 +24,7 @@ func (p *Pod) startInitContainers(ctx context.Context) error { } // Now iterate init containers for _, initCon := range initCtrs { - if err := initCon.Start(ctx, true); err != nil { + if err := initCon.startNoPodLock(ctx, true); err != nil { return err } // Check that the init container waited correctly and the exit