From a1eb6029cc4e4ccd820b0aac147a6121f1967ba2 Mon Sep 17 00:00:00 2001
From: Michael Crosby <crosbymichael@gmail.com>
Date: Wed, 20 Jan 2016 13:18:36 -0800
Subject: [PATCH] Move tty set and restore to caller

Fixes #19506

This fixes the issue of errors on create and the tty not being able to
be restored to its previous state because of a race where it was
in the hijack goroutine.

Signed-off-by: Michael Crosby <crosbymichael@gmail.com>
---
 api/client/attach.go |  6 ++++++
 api/client/cli.go    | 23 +++++++++++++++++++++++
 api/client/exec.go   |  6 ++++++
 api/client/hijack.go | 28 +++-------------------------
 api/client/run.go    |  6 ++++++
 api/client/start.go  |  6 ++++++
 6 files changed, 50 insertions(+), 25 deletions(-)

diff --git a/api/client/attach.go b/api/client/attach.go
index 6bfe1be187..efdad108ce 100644
--- a/api/client/attach.go
+++ b/api/client/attach.go
@@ -75,6 +75,12 @@ func (cli *DockerCli) CmdAttach(args ...string) error {
 		return err
 	}
 	defer resp.Close()
+	if in != nil && c.Config.Tty {
+		if err := cli.setRawTerminal(); err != nil {
+			return err
+		}
+		defer cli.restoreTerminal(in)
+	}
 
 	if err := cli.holdHijackedConnection(c.Config.Tty, in, cli.out, cli.err, resp); err != nil {
 		return err
diff --git a/api/client/cli.go b/api/client/cli.go
index 2ad3afabb0..b1637b0204 100644
--- a/api/client/cli.go
+++ b/api/client/cli.go
@@ -44,6 +44,8 @@ type DockerCli struct {
 	isTerminalOut bool
 	// client is the http client that performs all API operations
 	client client.APIClient
+	// state holds the terminal state
+	state *term.State
 }
 
 // Initialize calls the init function that will setup the configuration for the client
@@ -79,6 +81,27 @@ func (cli *DockerCli) ImagesFormat() string {
 	return cli.configFile.ImagesFormat
 }
 
+func (cli *DockerCli) setRawTerminal() error {
+	if cli.isTerminalIn && os.Getenv("NORAW") == "" {
+		state, err := term.SetRawTerminal(cli.inFd)
+		if err != nil {
+			return err
+		}
+		cli.state = state
+	}
+	return nil
+}
+
+func (cli *DockerCli) restoreTerminal(in io.Closer) error {
+	if cli.state != nil {
+		term.RestoreTerminal(cli.inFd, cli.state)
+	}
+	if in != nil {
+		return in.Close()
+	}
+	return nil
+}
+
 // NewDockerCli returns a DockerCli instance with IO output and error streams set by in, out and err.
 // The key file, protocol (i.e. unix) and address are passed in as strings, along with the tls.Config. If the tls.Config
 // is set the client scheme will be set to https.
diff --git a/api/client/exec.go b/api/client/exec.go
index 0ce9e81fe3..68b7c16e5d 100644
--- a/api/client/exec.go
+++ b/api/client/exec.go
@@ -87,6 +87,12 @@ func (cli *DockerCli) CmdExec(args ...string) error {
 		return err
 	}
 	defer resp.Close()
+	if in != nil && execConfig.Tty {
+		if err := cli.setRawTerminal(); err != nil {
+			return err
+		}
+		defer cli.restoreTerminal(in)
+	}
 	errCh = promise.Go(func() error {
 		return cli.holdHijackedConnection(execConfig.Tty, in, out, stderr, resp)
 	})
diff --git a/api/client/hijack.go b/api/client/hijack.go
index ea4b5e3875..4c80fe1cd9 100644
--- a/api/client/hijack.go
+++ b/api/client/hijack.go
@@ -2,41 +2,19 @@ package client
 
 import (
 	"io"
-	"os"
 
 	"github.com/Sirupsen/logrus"
 	"github.com/docker/docker/pkg/stdcopy"
-	"github.com/docker/docker/pkg/term"
 	"github.com/docker/engine-api/types"
 )
 
-func (cli *DockerCli) holdHijackedConnection(setRawTerminal bool, inputStream io.ReadCloser, outputStream, errorStream io.Writer, resp types.HijackedResponse) error {
-	var (
-		err      error
-		oldState *term.State
-	)
-	if inputStream != nil && setRawTerminal && cli.isTerminalIn && os.Getenv("NORAW") == "" {
-		oldState, err = term.SetRawTerminal(cli.inFd)
-		if err != nil {
-			return err
-		}
-		defer term.RestoreTerminal(cli.inFd, oldState)
-	}
-
+func (cli *DockerCli) holdHijackedConnection(tty bool, inputStream io.ReadCloser, outputStream, errorStream io.Writer, resp types.HijackedResponse) error {
+	var err error
 	receiveStdout := make(chan error, 1)
 	if outputStream != nil || errorStream != nil {
 		go func() {
-			defer func() {
-				if inputStream != nil {
-					if setRawTerminal && cli.isTerminalIn {
-						term.RestoreTerminal(cli.inFd, oldState)
-					}
-					inputStream.Close()
-				}
-			}()
-
 			// When TTY is ON, use regular copy
-			if setRawTerminal && outputStream != nil {
+			if tty && outputStream != nil {
 				_, err = io.Copy(outputStream, resp.Reader)
 			} else {
 				_, err = stdcopy.StdCopy(outputStream, errorStream, resp.Reader)
diff --git a/api/client/run.go b/api/client/run.go
index 3b3a1a2c77..16f4230e97 100644
--- a/api/client/run.go
+++ b/api/client/run.go
@@ -207,6 +207,12 @@ func (cli *DockerCli) CmdRun(args ...string) error {
 		if err != nil {
 			return err
 		}
+		if in != nil && config.Tty {
+			if err := cli.setRawTerminal(); err != nil {
+				return err
+			}
+			defer cli.restoreTerminal(in)
+		}
 		errCh = promise.Go(func() error {
 			return cli.holdHijackedConnection(config.Tty, in, out, stderr, resp)
 		})
diff --git a/api/client/start.go b/api/client/start.go
index 3a2bc12636..0d44217d19 100644
--- a/api/client/start.go
+++ b/api/client/start.go
@@ -96,6 +96,12 @@ func (cli *DockerCli) CmdStart(args ...string) error {
 			return err
 		}
 		defer resp.Close()
+		if in != nil && c.Config.Tty {
+			if err := cli.setRawTerminal(); err != nil {
+				return err
+			}
+			defer cli.restoreTerminal(in)
+		}
 
 		cErr := promise.Go(func() error {
 			return cli.holdHijackedConnection(c.Config.Tty, in, cli.out, cli.err, resp)