From 3dbfb3d38c4dcfb602e12ed2828d22d155897518 Mon Sep 17 00:00:00 2001 From: Brian Goff Date: Mon, 14 Sep 2015 16:34:23 -0400 Subject: [PATCH] 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 --- api/client/hijack.go | 46 ++++++++++++-------------- integration-cli/docker_cli_run_test.go | 21 ++++++++++++ 2 files changed, 42 insertions(+), 25 deletions(-) diff --git a/api/client/hijack.go b/api/client/hijack.go index a65468074c..2fcb9a5278 100644 --- a/api/client/hijack.go +++ b/api/client/hijack.go @@ -16,7 +16,6 @@ import ( "github.com/Sirupsen/logrus" "github.com/docker/docker/api" "github.com/docker/docker/autogen/dockerversion" - "github.com/docker/docker/pkg/promise" "github.com/docker/docker/pkg/stdcopy" "github.com/docker/docker/pkg/term" ) @@ -184,8 +183,6 @@ func (cli *DockerCli) hijack(method, path string, setRawTerminal bool, in io.Rea started <- rwc } - var receiveStdout chan error - var oldState *term.State 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 defer term.RestoreTerminal(cli.inFd, oldState) } + receiveStdout := make(chan error, 1) if stdout != nil || stderr != nil { - receiveStdout = promise.Go(func() (err error) { + go func() { defer func() { if in != nil { if setRawTerminal && cli.isTerminalIn { term.RestoreTerminal(cli.inFd, oldState) } - // For some reason this Close call blocks on darwin.. - // As the client exists right after, simply discard the close - // until we find a better solution. - if runtime.GOOS != "darwin" { - in.Close() - } + in.Close() } }() @@ -219,11 +212,12 @@ func (cli *DockerCli) hijack(method, path string, setRawTerminal bool, in io.Rea _, err = stdcopy.StdCopy(stdout, stderr, br) } logrus.Debugf("[hijack] End of stdout") - return err - }) + receiveStdout <- err + }() } - sendStdin := promise.Go(func() error { + stdinDone := make(chan struct{}) + go func() { if in != nil { io.Copy(rwc, in) logrus.Debugf("[hijack] End of stdin") @@ -236,22 +230,24 @@ func (cli *DockerCli) hijack(method, path string, setRawTerminal bool, in io.Rea logrus.Debugf("Couldn't send EOF: %s", err) } } - // Discard errors due to pipe interruption - return nil - }) + close(stdinDone) + }() - if stdout != nil || stderr != nil { - if err := <-receiveStdout; err != nil { + select { + case err := <-receiveStdout: + if err != nil { logrus.Debugf("Error receiveStdout: %s", err) - return err + } + if cli.isTerminalIn { + return nil + } + case <-stdinDone: + if stdout != nil || stderr != nil { + if err := <-receiveStdout; err != nil { + logrus.Debugf("Error receiveStdout: %s", err) + } } } - if !cli.isTerminalIn { - if err := <-sendStdin; err != nil { - logrus.Debugf("Error sendStdin: %s", err) - return err - } - } return nil } diff --git a/integration-cli/docker_cli_run_test.go b/integration-cli/docker_cli_run_test.go index cbc08b6834..f250d26910 100644 --- a/integration-cli/docker_cli_run_test.go +++ b/integration-cli/docker_cli_run_test.go @@ -3116,3 +3116,24 @@ func (s *DockerSuite) TestTwoContainersInNetHost(c *check.C) { dockerCmd(c, "stop", "first") dockerCmd(c, "stop", "second") } + +// #11957 - stdin with no tty does not exit if stdin is not closed even though container exited +func (s *DockerSuite) TestRunStdinBlockedAfterContainerExit(c *check.C) { + cmd := exec.Command(dockerBinary, "run", "-i", "--name=test", "busybox", "true") + in, err := cmd.StdinPipe() + c.Assert(err, check.IsNil) + defer in.Close() + c.Assert(cmd.Start(), check.IsNil) + + waitChan := make(chan error) + go func() { + waitChan <- cmd.Wait() + }() + + select { + case err := <-waitChan: + c.Assert(err, check.IsNil) + case <-time.After(3 * time.Second): + c.Fatal("timeout waiting for command to exit") + } +}