From d83d0abfbf6f914303c1b33d954aa115accbd999 Mon Sep 17 00:00:00 2001 From: Matthew Heon Date: Tue, 14 May 2019 14:54:21 -0400 Subject: 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 --- libpod/container_api.go | 12 ++++++++++-- libpod/container_attach_linux.go | 17 +++++++++++++---- libpod/container_attach_unsupported.go | 4 +++- 3 files changed, 26 insertions(+), 7 deletions(-) (limited to 'libpod') 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 diff --git a/libpod/container_attach_linux.go b/libpod/container_attach_linux.go index 3ff6ddc76..7e9b7697b 100644 --- a/libpod/container_attach_linux.go +++ b/libpod/container_attach_linux.go @@ -8,6 +8,7 @@ import ( "net" "os" "path/filepath" + "sync" "github.com/containers/libpod/pkg/kubeutils" "github.com/containers/libpod/utils" @@ -31,7 +32,7 @@ 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) error { +func (c *Container) attach(streams *AttachStreams, keys string, resize <-chan remotecommand.TerminalSize, startContainer bool, wg *sync.WaitGroup) error { if !streams.AttachOutput && !streams.AttachError && !streams.AttachInput { return errors.Wrapf(ErrInvalidArg, "must provide at least one stream to attach to") } @@ -48,12 +49,17 @@ 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, startContainer) + return c.attachContainerSocket(resize, detachKeys, streams, startContainer, wg) } -// 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 // TODO add a channel to allow interrupting -func (c *Container) attachContainerSocket(resize <-chan remotecommand.TerminalSize, detachKeys []byte, streams *AttachStreams, startContainer bool) error { +func (c *Container) attachContainerSocket(resize <-chan remotecommand.TerminalSize, detachKeys []byte, streams *AttachStreams, startContainer bool, wg *sync.WaitGroup) error { + if startContainer && wg == nil { + return errors.Wrapf(ErrInternal, "wait group not passed when startContainer set") + } + kubeutils.HandleResizing(resize, func(size remotecommand.TerminalSize) { controlPath := filepath.Join(c.bundlePath(), "ctl") controlFile, err := os.OpenFile(controlPath, unix.O_WRONLY, 0) @@ -84,10 +90,13 @@ func (c *Container) attachContainerSocket(resize <-chan remotecommand.TerminalSi } defer conn.Close() + // If starting was requested, start the container and notify when that's + // done. if startContainer { if err := c.start(); err != nil { return err } + wg.Done() } receiveStdoutError := make(chan error) diff --git a/libpod/container_attach_unsupported.go b/libpod/container_attach_unsupported.go index 068652b29..9e8badeaf 100644 --- a/libpod/container_attach_unsupported.go +++ b/libpod/container_attach_unsupported.go @@ -3,9 +3,11 @@ package libpod import ( + "sync" + "k8s.io/client-go/tools/remotecommand" ) -func (c *Container) attach(streams *AttachStreams, keys string, resize <-chan remotecommand.TerminalSize, startContainer bool) error { +func (c *Container) attach(streams *AttachStreams, keys string, resize <-chan remotecommand.TerminalSize, startContainer bool, wg *sync.WaitGroup) error { return ErrNotImplemented } -- cgit v1.2.3-54-g00ecf