Browse code

Merge pull request #26744 from LK4D4/attach_before_start

libcontainerd: attach streams before create

Victor Vieux authored on 2016/09/27 05:08:05
Showing 2 changed files
... ...
@@ -4507,3 +4507,15 @@ func (s *DockerDaemonSuite) TestRunWithUlimitAndDaemonDefault(c *check.C) {
4507 4507
 	c.Assert(err, checker.IsNil)
4508 4508
 	c.Assert(out, checker.Contains, "[nofile=42:42]")
4509 4509
 }
4510
+
4511
+func (s *DockerSuite) TestRunStoppedLoggingDriverNoLeak(c *check.C) {
4512
+	nroutines, err := getGoroutineNumber()
4513
+	c.Assert(err, checker.IsNil)
4514
+
4515
+	out, _, err := dockerCmdWithError("run", "--name=fail", "--log-driver=splunk", "busybox", "true")
4516
+	c.Assert(err, checker.NotNil)
4517
+	c.Assert(out, checker.Contains, "Failed to initialize logging driver", check.Commentf("error should be about logging driver, got output %s", out))
4518
+
4519
+	// NGoroutines is not updated right away, so we need to wait before failing
4520
+	c.Assert(waitForGoroutines(nroutines), checker.IsNil)
4521
+}
... ...
@@ -11,6 +11,7 @@ import (
11 11
 
12 12
 	"github.com/Sirupsen/logrus"
13 13
 	containerd "github.com/docker/containerd/api/grpc/types"
14
+	"github.com/docker/docker/pkg/ioutils"
14 15
 	"github.com/docker/docker/restartmanager"
15 16
 	"github.com/opencontainers/runtime-spec/specs-go"
16 17
 	"golang.org/x/net/context"
... ...
@@ -91,11 +92,24 @@ func (ctr *container) start(checkpoint string, checkpointDir string) error {
91 91
 	if err != nil {
92 92
 		return nil
93 93
 	}
94
+	createChan := make(chan struct{})
94 95
 	iopipe, err := ctr.openFifos(spec.Process.Terminal)
95 96
 	if err != nil {
96 97
 		return err
97 98
 	}
98 99
 
100
+	// we need to delay stdin closure after container start or else "stdin close"
101
+	// event will be rejected by containerd.
102
+	// stdin closure happens in AttachStreams
103
+	stdin := iopipe.Stdin
104
+	iopipe.Stdin = ioutils.NewWriteCloserWrapper(stdin, func() error {
105
+		go func() {
106
+			<-createChan
107
+			stdin.Close()
108
+		}()
109
+		return nil
110
+	})
111
+
99 112
 	r := &containerd.CreateContainerRequest{
100 113
 		Id:            ctr.containerID,
101 114
 		BundlePath:    ctr.dir,
... ...
@@ -111,17 +125,21 @@ func (ctr *container) start(checkpoint string, checkpointDir string) error {
111 111
 	}
112 112
 	ctr.client.appendContainer(ctr)
113 113
 
114
-	resp, err := ctr.client.remote.apiClient.CreateContainer(context.Background(), r)
115
-	if err != nil {
114
+	if err := ctr.client.backend.AttachStreams(ctr.containerID, *iopipe); err != nil {
115
+		close(createChan)
116 116
 		ctr.closeFifos(iopipe)
117 117
 		return err
118 118
 	}
119
-	ctr.startedAt = time.Now()
120 119
 
121
-	if err := ctr.client.backend.AttachStreams(ctr.containerID, *iopipe); err != nil {
120
+	resp, err := ctr.client.remote.apiClient.CreateContainer(context.Background(), r)
121
+	if err != nil {
122
+		close(createChan)
123
+		ctr.closeFifos(iopipe)
122 124
 		return err
123 125
 	}
126
+	ctr.startedAt = time.Now()
124 127
 	ctr.systemPid = systemPid(resp.Container)
128
+	close(createChan)
125 129
 
126 130
 	return ctr.client.backend.StateChanged(ctr.containerID, StateInfo{
127 131
 		CommonStateInfo: CommonStateInfo{