From 02d1934279294f28af6e509a29f909654677ed8b Mon Sep 17 00:00:00 2001 From: Alexander Morozov Date: Tue, 20 Sep 2016 08:10:04 -0700 Subject: [PATCH] libcontainerd: attach streams before create Fix #26371 Signed-off-by: Alexander Morozov --- integration-cli/docker_cli_run_test.go | 12 ++++++++++++ libcontainerd/container_linux.go | 26 ++++++++++++++++++++++---- 2 files changed, 34 insertions(+), 4 deletions(-) diff --git a/integration-cli/docker_cli_run_test.go b/integration-cli/docker_cli_run_test.go index 0ecfd08a91..d83c086743 100644 --- a/integration-cli/docker_cli_run_test.go +++ b/integration-cli/docker_cli_run_test.go @@ -4512,3 +4512,15 @@ func (s *DockerDaemonSuite) TestRunWithUlimitAndDaemonDefault(c *check.C) { c.Assert(err, checker.IsNil) c.Assert(out, checker.Contains, "[nofile=42:42]") } + +func (s *DockerSuite) TestRunStoppedLoggingDriverNoLeak(c *check.C) { + nroutines, err := getGoroutineNumber() + c.Assert(err, checker.IsNil) + + out, _, err := dockerCmdWithError("run", "--name=fail", "--log-driver=splunk", "busybox", "true") + c.Assert(err, checker.NotNil) + c.Assert(out, checker.Contains, "Failed to initialize logging driver", check.Commentf("error should be about logging driver, got output %s", out)) + + // NGoroutines is not updated right away, so we need to wait before failing + c.Assert(waitForGoroutines(nroutines), checker.IsNil) +} diff --git a/libcontainerd/container_linux.go b/libcontainerd/container_linux.go index 68344c551b..25e2cefa5e 100644 --- a/libcontainerd/container_linux.go +++ b/libcontainerd/container_linux.go @@ -11,6 +11,7 @@ import ( "github.com/Sirupsen/logrus" containerd "github.com/docker/containerd/api/grpc/types" + "github.com/docker/docker/pkg/ioutils" "github.com/docker/docker/restartmanager" "github.com/opencontainers/runtime-spec/specs-go" "golang.org/x/net/context" @@ -91,11 +92,24 @@ func (ctr *container) start(checkpoint string, checkpointDir string) error { if err != nil { return nil } + createChan := make(chan struct{}) iopipe, err := ctr.openFifos(spec.Process.Terminal) if err != nil { return err } + // we need to delay stdin closure after container start or else "stdin close" + // event will be rejected by containerd. + // stdin closure happens in AttachStreams + stdin := iopipe.Stdin + iopipe.Stdin = ioutils.NewWriteCloserWrapper(stdin, func() error { + go func() { + <-createChan + stdin.Close() + }() + return nil + }) + r := &containerd.CreateContainerRequest{ Id: ctr.containerID, BundlePath: ctr.dir, @@ -111,17 +125,21 @@ func (ctr *container) start(checkpoint string, checkpointDir string) error { } ctr.client.appendContainer(ctr) + if err := ctr.client.backend.AttachStreams(ctr.containerID, *iopipe); err != nil { + close(createChan) + ctr.closeFifos(iopipe) + return err + } + resp, err := ctr.client.remote.apiClient.CreateContainer(context.Background(), r) if err != nil { + close(createChan) ctr.closeFifos(iopipe) return err } ctr.startedAt = time.Now() - - if err := ctr.client.backend.AttachStreams(ctr.containerID, *iopipe); err != nil { - return err - } ctr.systemPid = systemPid(resp.Container) + close(createChan) return ctr.client.backend.StateChanged(ctr.containerID, StateInfo{ CommonStateInfo: CommonStateInfo{