From da44d8d5c839c7e09714495a7b121d8aa8664413 Mon Sep 17 00:00:00 2001 From: Giuseppe Scrivano Date: Wed, 20 Jun 2018 09:36:01 +0200 Subject: 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 --- libpod/container_api.go | 10 ++-------- libpod/container_attach.go | 12 +++++++++--- 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() { -- cgit v1.2.3-54-g00ecf