libpod: fix hang on container start and attach

When a container is attached upon start, the WaitGroup counter may
never be decremented if an error is raised before start, causing
the caller to hang.
Synchronize with the start & attach goroutine using a channel, to be
able to detect failures before start.

Signed-off-by: Marco Vedovati <mvedovati@suse.com>

3 files changed, 18 insertions, 18 deletions

diff --git a/libpod/container_api.go b/libpod/container_api.go
index b8c339a39..d5d8e9906 100644
--- a/libpod/container_api.go
+++ b/libpod/container_api.go
@@ -7,7 +7,6 @@ import (
 	"io/ioutil"
 	"os"
 	"strconv"
-	"sync"
 	"time"
 
 	"github.com/containers/libpod/libpod/define"
@@ -120,20 +119,24 @@ 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)
+	// Use a channel to sync
+	startedChan := make(chan bool)
 
 	// Attach to the container before starting it
 	go func() {
-		if err := c.attach(streams, keys, resize, true, wg); err != nil {
+		if err := c.attach(streams, keys, resize, true, startedChan); err != nil {
 			attachChan <- err
 		}
 		close(attachChan)
 	}()
 
-	wg.Wait()
-	c.newContainerEvent(events.Attach)
+	select {
+	case err := <-attachChan:
+		return nil, err
+	case <-startedChan:
+		c.newContainerEvent(events.Attach)
+	}
+
 	return attachChan, nil
 }
 
diff --git a/libpod/container_attach_linux.go b/libpod/container_attach_linux.go
index 3de5ea1cf..f5aac5794 100644
--- a/libpod/container_attach_linux.go
+++ b/libpod/container_attach_linux.go
@@ -8,7 +8,6 @@ import (
 	"net"
 	"os"
 	"path/filepath"
-	"sync"
 
 	"github.com/containers/libpod/libpod/define"
 	"github.com/containers/libpod/pkg/kubeutils"
@@ -33,22 +32,22 @@ const (
 
 // Attach to the given container
 // Does not check if state is appropriate
-func (c *Container) attach(streams *AttachStreams, keys string, resize <-chan remotecommand.TerminalSize, startContainer bool, wg *sync.WaitGroup) error {
+func (c *Container) attach(streams *AttachStreams, keys string, resize <-chan remotecommand.TerminalSize, startContainer bool, started chan bool) error {
 	if !streams.AttachOutput && !streams.AttachError && !streams.AttachInput {
 		return errors.Wrapf(define.ErrInvalidArg, "must provide at least one stream to attach to")
 	}
 
 	logrus.Debugf("Attaching to container %s", c.ID())
 
-	return c.attachContainerSocket(resize, keys, streams, startContainer, wg)
+	return c.attachContainerSocket(resize, keys, streams, startContainer, started)
 }
 
 // attachContainerSocket connects to the container's attach socket and deals with the IO.
-// wg is only required if startContainer is true
+// started is only required if startContainer is true
 // TODO add a channel to allow interrupting
-func (c *Container) attachContainerSocket(resize <-chan remotecommand.TerminalSize, keys string, streams *AttachStreams, startContainer bool, wg *sync.WaitGroup) error {
-	if startContainer && wg == nil {
-		return errors.Wrapf(define.ErrInternal, "wait group not passed when startContainer set")
+func (c *Container) attachContainerSocket(resize <-chan remotecommand.TerminalSize, keys string, streams *AttachStreams, startContainer bool, started chan bool) error {
+	if startContainer && started == nil {
+		return errors.Wrapf(define.ErrInternal, "started chan not passed when startContainer set")
 	}
 
 	// Use default detach keys when keys aren't passed or specified in libpod.conf
@@ -100,7 +99,7 @@ func (c *Container) attachContainerSocket(resize <-chan remotecommand.TerminalSi
 		if err := c.start(); err != nil {
 			return err
 		}
-		wg.Done()
+		started <- true
 	}
 
 	receiveStdoutError := make(chan error)
diff --git a/libpod/container_attach_unsupported.go b/libpod/container_attach_unsupported.go
index 2c8718c67..c27ce0799 100644
--- a/libpod/container_attach_unsupported.go
+++ b/libpod/container_attach_unsupported.go
@@ -3,12 +3,10 @@
 package libpod
 
 import (
-	"sync"
-
 	"github.com/containers/libpod/libpod/define"
 	"k8s.io/client-go/tools/remotecommand"
 )
 
-func (c *Container) attach(streams *AttachStreams, keys string, resize <-chan remotecommand.TerminalSize, startContainer bool, wg *sync.WaitGroup) error {
+func (c *Container) attach(streams *AttachStreams, keys string, resize <-chan remotecommand.TerminalSize, startContainer bool, started chan bool) error {
 	return define.ErrNotImplemented
 }