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>
This commit is contained in:
Marco Vedovati
2019-06-17 15:14:54 +02:00
parent 6e9b490f5e
commit 4f56964d55
3 changed files with 18 additions and 18 deletions

View File

@ -7,7 +7,6 @@ import (
"io/ioutil" "io/ioutil"
"os" "os"
"strconv" "strconv"
"sync"
"time" "time"
"github.com/containers/libpod/libpod/define" "github.com/containers/libpod/libpod/define"
@ -120,20 +119,24 @@ func (c *Container) StartAndAttach(ctx context.Context, streams *AttachStreams,
attachChan := make(chan error) attachChan := make(chan error)
// We need to ensure that we don't return until start() fired in attach. // We need to ensure that we don't return until start() fired in attach.
// Use a WaitGroup to sync this. // Use a channel to sync
wg := new(sync.WaitGroup) startedChan := make(chan bool)
wg.Add(1)
// Attach to the container before starting it // Attach to the container before starting it
go func() { 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 attachChan <- err
} }
close(attachChan) close(attachChan)
}() }()
wg.Wait() select {
case err := <-attachChan:
return nil, err
case <-startedChan:
c.newContainerEvent(events.Attach) c.newContainerEvent(events.Attach)
}
return attachChan, nil return attachChan, nil
} }

View File

@ -8,7 +8,6 @@ import (
"net" "net"
"os" "os"
"path/filepath" "path/filepath"
"sync"
"github.com/containers/libpod/libpod/define" "github.com/containers/libpod/libpod/define"
"github.com/containers/libpod/pkg/kubeutils" "github.com/containers/libpod/pkg/kubeutils"
@ -33,22 +32,22 @@ const (
// Attach to the given container // Attach to the given container
// Does not check if state is appropriate // 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 { if !streams.AttachOutput && !streams.AttachError && !streams.AttachInput {
return errors.Wrapf(define.ErrInvalidArg, "must provide at least one stream to attach to") return errors.Wrapf(define.ErrInvalidArg, "must provide at least one stream to attach to")
} }
logrus.Debugf("Attaching to container %s", c.ID()) 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. // 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 // 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 { func (c *Container) attachContainerSocket(resize <-chan remotecommand.TerminalSize, keys string, streams *AttachStreams, startContainer bool, started chan bool) error {
if startContainer && wg == nil { if startContainer && started == nil {
return errors.Wrapf(define.ErrInternal, "wait group not passed when startContainer set") 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 // 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 { if err := c.start(); err != nil {
return err return err
} }
wg.Done() started <- true
} }
receiveStdoutError := make(chan error) receiveStdoutError := make(chan error)

View File

@ -3,12 +3,10 @@
package libpod package libpod
import ( import (
"sync"
"github.com/containers/libpod/libpod/define" "github.com/containers/libpod/libpod/define"
"k8s.io/client-go/tools/remotecommand" "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 return define.ErrNotImplemented
} }