Merge pull request #20589 from coolljt0725/fix_restore_terminal

cli: move setRawTerminal and restoreTerminal to holdHijackedConnection

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(c.Config.Tty, in, cli.out, cli.err, resp); err != nil {
+	if err := cli.holdHijackedConnection(context.Background(), 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 {
-	var err error
+func (cli *DockerCli) holdHijackedConnection(ctx context.Context, tty bool, inputStream io.ReadCloser, outputStream, errorStream io.Writer, resp types.HijackedResponse) 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,11 +77,16 @@ func (cli *DockerCli) holdHijackedConnection(tty bool, inputStream io.ReadCloser
 		}
 	case <-stdinDone:
 		if outputStream != nil || errorStream != nil {
-			if err := <-receiveStdout; err != nil {
-				logrus.Debugf("Error receiveStdout: %s", err)
-				return err
+			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

@@ -158,6 +158,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
@@ -170,8 +172,8 @@ func (cli *DockerCli) CmdRun(args ...string) error {
 	if *flAutoRemove && (hostConfig.RestartPolicy.IsAlways() || hostConfig.RestartPolicy.IsOnFailure()) {
 		return ErrConflictRestartPolicyAndAutoRemove
 	}
-
-	if config.AttachStdin || config.AttachStdout || config.AttachStderr {
+	attach := config.AttachStdin || config.AttachStdout || config.AttachStderr
+	if attach {
 		var (
 			out, stderr io.Writer
 			in          io.ReadCloser
@@ -207,14 +209,9 @@ 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)
-		}
+		ctx, cancelFun = context.WithCancel(context.Background())
 		errCh = promise.Go(func() error {
-			return cli.holdHijackedConnection(config.Tty, in, out, stderr, resp)
+			return cli.holdHijackedConnection(ctx, config.Tty, in, out, stderr, resp)
 		})
 	}
 
@@ -228,6 +225,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 {
-			if err := cli.setRawTerminal(); err != nil {
-				return err
-			}
-			defer cli.restoreTerminal(in)
-		}
-
+		ctx, cancelFun := context.WithCancel(context.Background())
 		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
 		}