libpod: fix race with attach/start

Move the StartContainer call after the attach to the UNIX socket.  It
solves a race where the StartContainer could be done earlier and a
short-lived container could already exit by the time we tried to
attach to the socket.

Closes: https://github.com/projectatomic/libpod/issues/835

Signed-off-by: Giuseppe Scrivano <gscrivan@redhat.com>

2 files changed, 11 insertions, 11 deletions

diff --git a/libpod/container_api.go b/libpod/container_api.go
index 0ddccfe8e..96648ccf5 100644
--- a/libpod/container_api.go
+++ b/libpod/container_api.go
@@ -178,18 +178,12 @@ func (c *Container) StartAndAttach(ctx context.Context, streams *AttachStreams,
 
 	// Attach to the container before starting it
 	go func() {
-		if err := c.attach(streams, keys, resize); err != nil {
+		if err := c.attach(streams, keys, resize, true); err != nil {
 			attachChan <- err
 		}
 		close(attachChan)
 	}()
 
-	// Start the container
-	if err := c.start(); err != nil {
-		// TODO: interrupt the attach here if we error
-		return nil, err
-	}
-
 	return attachChan, nil
 }
 
@@ -421,7 +415,7 @@ func (c *Container) Attach(streams *AttachStreams, keys string, resize <-chan re
 		return errors.Wrapf(ErrCtrStateInvalid, "can only attach to created or running containers")
 	}
 
-	return c.attach(streams, keys, resize)
+	return c.attach(streams, keys, resize, false)
 }
 
 // Mount mounts a container's filesystem on the host
diff --git a/libpod/container_attach.go b/libpod/container_attach.go
index ffe3a5edc..29e2d2fa6 100644
--- a/libpod/container_attach.go
+++ b/libpod/container_attach.go
@@ -44,7 +44,7 @@ type AttachStreams struct {
 
 // Attach to the given container
 // Does not check if state is appropriate
-func (c *Container) attach(streams *AttachStreams, keys string, resize <-chan remotecommand.TerminalSize) error {
+func (c *Container) attach(streams *AttachStreams, keys string, resize <-chan remotecommand.TerminalSize, startContainer bool) error {
 	if !streams.AttachOutput && !streams.AttachError && !streams.AttachInput {
 		return errors.Wrapf(ErrInvalidArg, "must provide at least one stream to attach to")
 	}
@@ -61,12 +61,12 @@ func (c *Container) attach(streams *AttachStreams, keys string, resize <-chan re
 
 	logrus.Debugf("Attaching to container %s", c.ID())
 
-	return c.attachContainerSocket(resize, detachKeys, streams)
+	return c.attachContainerSocket(resize, detachKeys, streams, startContainer)
 }
 
 // attachContainerSocket connects to the container's attach socket and deals with the IO
 // TODO add a channel to allow interrupting
-func (c *Container) attachContainerSocket(resize <-chan remotecommand.TerminalSize, detachKeys []byte, streams *AttachStreams) error {
+func (c *Container) attachContainerSocket(resize <-chan remotecommand.TerminalSize, detachKeys []byte, streams *AttachStreams, startContainer bool) error {
 	kubeutils.HandleResizing(resize, func(size remotecommand.TerminalSize) {
 		controlPath := filepath.Join(c.bundlePath(), "ctl")
 		controlFile, err := os.OpenFile(controlPath, unix.O_WRONLY, 0)
@@ -89,6 +89,12 @@ func (c *Container) attachContainerSocket(resize <-chan remotecommand.TerminalSi
 	}
 	defer conn.Close()
 
+	if startContainer {
+		if err := c.start(); err != nil {
+			return err
+		}
+	}
+
 	receiveStdoutError := make(chan error)
 	if streams.AttachOutput || streams.AttachError {
 		go func() {