cli: move setRawTerminal and restoreTerminal to holdHijackedConnection

In this way, we can restore the Terminal as soon as possible once the hijacked
connection end. This not only fix weird output if cli enable -D, but also
remove duplicate code.

Signed-off-by: Lei Jitang <leijitang@huawei.com>

api/client/attach.go

@@ -71,12 +71,6 @@ 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 c.Config.Tty && cli.isTerminalOut {
 		height, width := cli.getTtySize()
@@ -92,8 +86,7 @@ func (cli *DockerCli) CmdAttach(args ...string) error {
 			logrus.Debugf("Error monitoring TTY size: %s", err)
 		}
 	}
-
+	if err := cli.holdHijackedConnection(context.Background(), c.Config.Tty, in, cli.out, cli.err, resp); err != nil {
-	if err := cli.holdHijackedConnection(c.Config.Tty, in, cli.out, cli.err, resp); err != nil {
 		return err
 	}
 

api/client/exec.go

@@ -89,14 +89,8 @@ 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)
+		return cli.holdHijackedConnection(context.Background(), execConfig.Tty, in, out, stderr, resp)
 	})
 
 	if execConfig.Tty && cli.isTerminalIn {

api/client/hijack.go

@@ -2,23 +2,48 @@ package client
 
 import (
 	"io"
+	"sync"
+
+	"golang.org/x/net/context"
 
 	"github.com/Sirupsen/logrus"
 	"github.com/docker/docker/pkg/stdcopy"
 	"github.com/docker/engine-api/types"
 )
 
-func (cli *DockerCli) holdHijackedConnection(tty bool, inputStream io.ReadCloser, outputStream, errorStream io.Writer, resp types.HijackedResponse) error {
+func (cli *DockerCli) holdHijackedConnection(ctx context.Context, tty bool, inputStream io.ReadCloser, outputStream, errorStream io.Writer, resp types.HijackedResponse) error {
-	var err error
+	var (
+		err         error
+		restoreOnce sync.Once
+	)
+	if inputStream != nil && tty {
+		if err := cli.setRawTerminal(); err != nil {
+			return err
+		}
+		defer func() {
+			restoreOnce.Do(func() {
+				cli.restoreTerminal(inputStream)
+			})
+		}()
+	}
+
 	receiveStdout := make(chan error, 1)
 	if outputStream != nil || errorStream != nil {
 		go func() {
 			// When TTY is ON, use regular copy
 			if tty && outputStream != nil {
 				_, err = io.Copy(outputStream, resp.Reader)
+				// we should restore the terminal as soon as possible once connection end
+				// so any following print messages will be in normal type.
+				if inputStream != nil {
+					restoreOnce.Do(func() {
+						cli.restoreTerminal(inputStream)
+					})
+				}
 			} else {
 				_, err = stdcopy.StdCopy(outputStream, errorStream, resp.Reader)
 			}
+
 			logrus.Debugf("[hijack] End of stdout")
 			receiveStdout <- err
 		}()
@@ -28,6 +53,13 @@ func (cli *DockerCli) holdHijackedConnection(tty bool, inputStream io.ReadCloser
 	go func() {
 		if inputStream != nil {
 			io.Copy(resp.Conn, inputStream)
+			// we should restore the terminal as soon as possible once connection end
+			// so any following print messages will be in normal type.
+			if tty {
+				restoreOnce.Do(func() {
+					cli.restoreTerminal(inputStream)
+				})
+			}
 			logrus.Debugf("[hijack] End of stdin")
 		}
 
@@ -45,12 +77,17 @@ func (cli *DockerCli) holdHijackedConnection(tty bool, inputStream io.ReadCloser
 		}
 	case <-stdinDone:
 		if outputStream != nil || errorStream != nil {
-			if err := <-receiveStdout; err != nil {
+			select {
+			case err := <-receiveStdout:
+				if err != nil {
 					logrus.Debugf("Error receiveStdout: %s", err)
 					return err
 				}
+			case <-ctx.Done():
 			}
 		}
+	case <-ctx.Done():
+	}
 
 	return nil
 }

api/client/run.go

@@ -159,6 +159,8 @@ func (cli *DockerCli) CmdRun(args ...string) error {
 	var (
 		waitDisplayID chan struct{}
 		errCh         chan error
+		cancelFun     context.CancelFunc
+		ctx           context.Context
 	)
 	if !config.AttachStdout && !config.AttachStderr {
 		// Make this asynchronous to allow the client to write to stdin before having to read the ID
@@ -171,8 +173,8 @@ func (cli *DockerCli) CmdRun(args ...string) error {
 	if *flAutoRemove && (hostConfig.RestartPolicy.IsAlways() || hostConfig.RestartPolicy.IsOnFailure()) {
 		return ErrConflictRestartPolicyAndAutoRemove
 	}
-
+	attach := config.AttachStdin || config.AttachStdout || config.AttachStderr
-	if config.AttachStdin || config.AttachStdout || config.AttachStderr {
+	if attach {
 		var (
 			out, stderr io.Writer
 			in          io.ReadCloser
@@ -208,14 +210,9 @@ func (cli *DockerCli) CmdRun(args ...string) error {
 		if err != nil {
 			return err
 		}
-		if in != nil && config.Tty {
+		ctx, cancelFun = context.WithCancel(context.Background())
-			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)
+			return cli.holdHijackedConnection(ctx, config.Tty, in, out, stderr, resp)
 		})
 	}
 
@@ -229,6 +226,14 @@ func (cli *DockerCli) CmdRun(args ...string) error {
 
 	//start the container
 	if err := cli.client.ContainerStart(context.Background(), createResponse.ID); err != nil {
+		// If we have holdHijackedConnection, we should notify
+		// holdHijackedConnection we are going to exit and wait
+		// to avoid the terminal are not restored.
+		if attach {
+			cancelFun()
+			<-errCh
+		}
+
 		cmd.ReportError(err.Error(), false)
 		return runStartContainerErr(err)
 	}

api/client/start.go

@@ -89,6 +89,7 @@ func (cli *DockerCli) CmdStart(args ...string) error {
 		}
 
 		var in io.ReadCloser
+
 		if options.Stdin {
 			in = cli.in
 		}
@@ -98,19 +99,15 @@ func (cli *DockerCli) CmdStart(args ...string) error {
 			return err
 		}
 		defer resp.Close()
-		if in != nil && c.Config.Tty {
+		ctx, cancelFun := context.WithCancel(context.Background())
-			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)
+			return cli.holdHijackedConnection(ctx, c.Config.Tty, in, cli.out, cli.err, resp)
 		})
 
 		// 3. Start the container.
 		if err := cli.client.ContainerStart(context.Background(), containerID); err != nil {
+			cancelFun()
+			<-cErr
 			return err
 		}