Browse code

Ensure stdin does not block after container stop

Fixes #11957
Fixes #12319

Also removes check for Darwin when the stdin reader is closed as it
doesn't appear to block any more.

Signed-off-by: Brian Goff <cpuguy83@gmail.com>

Brian Goff authored on 2015/09/15 05:34:23
Showing 2 changed files
... ...
@@ -16,7 +16,6 @@ import (
16 16
 	"github.com/Sirupsen/logrus"
17 17
 	"github.com/docker/docker/api"
18 18
 	"github.com/docker/docker/autogen/dockerversion"
19
-	"github.com/docker/docker/pkg/promise"
20 19
 	"github.com/docker/docker/pkg/stdcopy"
21 20
 	"github.com/docker/docker/pkg/term"
22 21
 )
... ...
@@ -184,8 +183,6 @@ func (cli *DockerCli) hijack(method, path string, setRawTerminal bool, in io.Rea
184 184
 		started <- rwc
185 185
 	}
186 186
 
187
-	var receiveStdout chan error
188
-
189 187
 	var oldState *term.State
190 188
 
191 189
 	if in != nil && setRawTerminal && cli.isTerminalIn && os.Getenv("NORAW") == "" {
... ...
@@ -196,19 +193,15 @@ func (cli *DockerCli) hijack(method, path string, setRawTerminal bool, in io.Rea
196 196
 		defer term.RestoreTerminal(cli.inFd, oldState)
197 197
 	}
198 198
 
199
+	receiveStdout := make(chan error, 1)
199 200
 	if stdout != nil || stderr != nil {
200
-		receiveStdout = promise.Go(func() (err error) {
201
+		go func() {
201 202
 			defer func() {
202 203
 				if in != nil {
203 204
 					if setRawTerminal && cli.isTerminalIn {
204 205
 						term.RestoreTerminal(cli.inFd, oldState)
205 206
 					}
206
-					// For some reason this Close call blocks on darwin..
207
-					// As the client exists right after, simply discard the close
208
-					// until we find a better solution.
209
-					if runtime.GOOS != "darwin" {
210
-						in.Close()
211
-					}
207
+					in.Close()
212 208
 				}
213 209
 			}()
214 210
 
... ...
@@ -219,11 +212,12 @@ func (cli *DockerCli) hijack(method, path string, setRawTerminal bool, in io.Rea
219 219
 				_, err = stdcopy.StdCopy(stdout, stderr, br)
220 220
 			}
221 221
 			logrus.Debugf("[hijack] End of stdout")
222
-			return err
223
-		})
222
+			receiveStdout <- err
223
+		}()
224 224
 	}
225 225
 
226
-	sendStdin := promise.Go(func() error {
226
+	stdinDone := make(chan struct{})
227
+	go func() {
227 228
 		if in != nil {
228 229
 			io.Copy(rwc, in)
229 230
 			logrus.Debugf("[hijack] End of stdin")
... ...
@@ -236,22 +230,24 @@ func (cli *DockerCli) hijack(method, path string, setRawTerminal bool, in io.Rea
236 236
 				logrus.Debugf("Couldn't send EOF: %s", err)
237 237
 			}
238 238
 		}
239
-		// Discard errors due to pipe interruption
240
-		return nil
241
-	})
239
+		close(stdinDone)
240
+	}()
242 241
 
243
-	if stdout != nil || stderr != nil {
244
-		if err := <-receiveStdout; err != nil {
242
+	select {
243
+	case err := <-receiveStdout:
244
+		if err != nil {
245 245
 			logrus.Debugf("Error receiveStdout: %s", err)
246
-			return err
247 246
 		}
248
-	}
249
-
250
-	if !cli.isTerminalIn {
251
-		if err := <-sendStdin; err != nil {
252
-			logrus.Debugf("Error sendStdin: %s", err)
253
-			return err
247
+		if cli.isTerminalIn {
248
+			return nil
249
+		}
250
+	case <-stdinDone:
251
+		if stdout != nil || stderr != nil {
252
+			if err := <-receiveStdout; err != nil {
253
+				logrus.Debugf("Error receiveStdout: %s", err)
254
+			}
254 255
 		}
255 256
 	}
257
+
256 258
 	return nil
257 259
 }
... ...
@@ -3116,3 +3116,24 @@ func (s *DockerSuite) TestTwoContainersInNetHost(c *check.C) {
3116 3116
 	dockerCmd(c, "stop", "first")
3117 3117
 	dockerCmd(c, "stop", "second")
3118 3118
 }
3119
+
3120
+// #11957 - stdin with no tty does not exit if stdin is not closed even though container exited
3121
+func (s *DockerSuite) TestRunStdinBlockedAfterContainerExit(c *check.C) {
3122
+	cmd := exec.Command(dockerBinary, "run", "-i", "--name=test", "busybox", "true")
3123
+	in, err := cmd.StdinPipe()
3124
+	c.Assert(err, check.IsNil)
3125
+	defer in.Close()
3126
+	c.Assert(cmd.Start(), check.IsNil)
3127
+
3128
+	waitChan := make(chan error)
3129
+	go func() {
3130
+		waitChan <- cmd.Wait()
3131
+	}()
3132
+
3133
+	select {
3134
+	case err := <-waitChan:
3135
+		c.Assert(err, check.IsNil)
3136
+	case <-time.After(3 * time.Second):
3137
+		c.Fatal("timeout waiting for command to exit")
3138
+	}
3139
+}