Browse Source

libcontainerd: attach streams before create

Fix #26371

Signed-off-by: Alexander Morozov <lk4d4@docker.com>
(cherry picked from commit 02d1934279294f28af6e509a29f909654677ed8b)
Alexander Morozov 8 years ago
parent
commit
c17155fbc3
2 changed files with 34 additions and 4 deletions
  1. 12 0
      integration-cli/docker_cli_run_test.go
  2. 22 4
      libcontainerd/container_linux.go

+ 12 - 0
integration-cli/docker_cli_run_test.go

@@ -4455,3 +4455,15 @@ func (s *DockerSuite) TestRunAddHostInHostMode(c *check.C) {
 	out, _ := dockerCmd(c, "run", "--add-host=extra:1.2.3.4", "--net=host", "busybox", "cat", "/etc/hosts")
 	out, _ := dockerCmd(c, "run", "--add-host=extra:1.2.3.4", "--net=host", "busybox", "cat", "/etc/hosts")
 	c.Assert(out, checker.Contains, expectedOutput, check.Commentf("Expected '%s', but got %q", expectedOutput, out))
 	c.Assert(out, checker.Contains, expectedOutput, check.Commentf("Expected '%s', but got %q", expectedOutput, out))
 }
 }
+
+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)
+}

+ 22 - 4
libcontainerd/container_linux.go

@@ -11,6 +11,7 @@ import (
 
 
 	"github.com/Sirupsen/logrus"
 	"github.com/Sirupsen/logrus"
 	containerd "github.com/docker/containerd/api/grpc/types"
 	containerd "github.com/docker/containerd/api/grpc/types"
+	"github.com/docker/docker/pkg/ioutils"
 	"github.com/docker/docker/restartmanager"
 	"github.com/docker/docker/restartmanager"
 	"github.com/opencontainers/specs/specs-go"
 	"github.com/opencontainers/specs/specs-go"
 	"github.com/tonistiigi/fifo"
 	"github.com/tonistiigi/fifo"
@@ -92,11 +93,24 @@ func (ctr *container) start() error {
 	if err != nil {
 	if err != nil {
 		return nil
 		return nil
 	}
 	}
+	createChan := make(chan struct{})
 	iopipe, err := ctr.openFifos(spec.Process.Terminal)
 	iopipe, err := ctr.openFifos(spec.Process.Terminal)
 	if err != nil {
 	if err != nil {
 		return err
 		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{
 	r := &containerd.CreateContainerRequest{
 		Id:         ctr.containerID,
 		Id:         ctr.containerID,
 		BundlePath: ctr.dir,
 		BundlePath: ctr.dir,
@@ -110,17 +124,21 @@ func (ctr *container) start() error {
 	}
 	}
 	ctr.client.appendContainer(ctr)
 	ctr.client.appendContainer(ctr)
 
 
-	resp, err := ctr.client.remote.apiClient.CreateContainer(context.Background(), r)
-	if err != nil {
+	if err := ctr.client.backend.AttachStreams(ctr.containerID, *iopipe); err != nil {
+		close(createChan)
 		ctr.closeFifos(iopipe)
 		ctr.closeFifos(iopipe)
 		return err
 		return err
 	}
 	}
-	ctr.startedAt = time.Now()
 
 
-	if err := ctr.client.backend.AttachStreams(ctr.containerID, *iopipe); err != nil {
+	resp, err := ctr.client.remote.apiClient.CreateContainer(context.Background(), r)
+	if err != nil {
+		close(createChan)
+		ctr.closeFifos(iopipe)
 		return err
 		return err
 	}
 	}
+	ctr.startedAt = time.Now()
 	ctr.systemPid = systemPid(resp.Container)
 	ctr.systemPid = systemPid(resp.Container)
+	close(createChan)
 
 
 	return ctr.client.backend.StateChanged(ctr.containerID, StateInfo{
 	return ctr.client.backend.StateChanged(ctr.containerID, StateInfo{
 		CommonStateInfo: CommonStateInfo{
 		CommonStateInfo: CommonStateInfo{