Ensure that start() in StartAndAttach() is locked

StartAndAttach() runs start() in a goroutine, which can allow it
to fire after the caller returns - and thus, after the defer to
unlock the container lock has fired.

The start() call _must_ occur while the container is locked, or
else state inconsistencies may occur.

Fixes #3114

Signed-off-by: Matthew Heon <matthew.heon@pm.me>

1 files changed, 10 insertions, 2 deletions

diff --git a/libpod/container_api.go b/libpod/container_api.go
index 5bb610aab..06a31da11 100644
--- a/libpod/container_api.go
+++ b/libpod/container_api.go
@@ -7,6 +7,7 @@ import (
 	"io/ioutil"
 	"os"
 	"strconv"
+	"sync"
 	"time"
 
 	"github.com/containers/libpod/libpod/driver"
@@ -119,13 +120,20 @@ func (c *Container) StartAndAttach(ctx context.Context, streams *AttachStreams,
 
 	attachChan := make(chan error)
 
+	// We need to ensure that we don't return until start() fired in attach.
+	// Use a WaitGroup to sync this.
+	wg := new(sync.WaitGroup)
+	wg.Add(1)
+
 	// Attach to the container before starting it
 	go func() {
-		if err := c.attach(streams, keys, resize, true); err != nil {
+		if err := c.attach(streams, keys, resize, true, wg); err != nil {
 			attachChan <- err
 		}
 		close(attachChan)
 	}()
+
+	wg.Wait()
 	c.newContainerEvent(events.Attach)
 	return attachChan, nil
 }
@@ -398,7 +406,7 @@ func (c *Container) Attach(streams *AttachStreams, keys string, resize <-chan re
 		return errors.Wrapf(ErrCtrStateInvalid, "can only attach to created or running containers")
 	}
 	defer c.newContainerEvent(events.Attach)
-	return c.attach(streams, keys, resize, false)
+	return c.attach(streams, keys, resize, false, nil)
 }
 
 // Mount mounts a container's filesystem on the host