1. docker
  2. Commit a1eb6029cc4e4ccd820b0aac147a6121f1967ba2
Browse code

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>

Michael Crosby authored on 2016/01/21 06:18:36
Showing 6 changed files
api/client/attach.go
History View file @ a1eb602
... ... 
@@ -75,6 +75,12 @@ func (cli *DockerCli) CmdAttach(args ...string) error {
75 75 
 		return err
76 76 
 	}
77 77 
 	defer resp.Close()
78 
+	if in != nil && c.Config.Tty {
79 
+		if err := cli.setRawTerminal(); err != nil {
80 
+			return err
81 
+		}
82 
+		defer cli.restoreTerminal(in)
83 
+	}
78 84
79 85 
 	if err := cli.holdHijackedConnection(c.Config.Tty, in, cli.out, cli.err, resp); err != nil {
80 86 
 		return err
api/client/cli.go
History View file @ a1eb602
... ... 
@@ -44,6 +44,8 @@ type DockerCli struct {
44 44 
 	isTerminalOut bool
45 45 
 	// client is the http client that performs all API operations
46 46 
 	client client.APIClient
47 
+	// state holds the terminal state
48 
+	state *term.State
47 49 
 }
48 50
49 51 
 // Initialize calls the init function that will setup the configuration for the client
... ... 
@@ -79,6 +81,27 @@ func (cli *DockerCli) ImagesFormat() string {
79 79 
 	return cli.configFile.ImagesFormat
80 80 
 }
81 81
82 
+func (cli *DockerCli) setRawTerminal() error {
83 
+	if cli.isTerminalIn && os.Getenv("NORAW") == "" {
84 
+		state, err := term.SetRawTerminal(cli.inFd)
85 
+		if err != nil {
86 
+			return err
87 
+		}
88 
+		cli.state = state
89 
+	}
90 
+	return nil
91 
+}
92 
+
93 
+func (cli *DockerCli) restoreTerminal(in io.Closer) error {
94 
+	if cli.state != nil {
95 
+		term.RestoreTerminal(cli.inFd, cli.state)
96 
+	}
97 
+	if in != nil {
98 
+		return in.Close()
99 
+	}
100 
+	return nil
101 
+}
102 
+
82 103 
 // NewDockerCli returns a DockerCli instance with IO output and error streams set by in, out and err.
83 104 
 // The key file, protocol (i.e. unix) and address are passed in as strings, along with the tls.Config. If the tls.Config
84 105 
 // is set the client scheme will be set to https.
api/client/exec.go
History View file @ a1eb602
... ... 
@@ -87,6 +87,12 @@ func (cli *DockerCli) CmdExec(args ...string) error {
87 87 
 		return err
88 88 
 	}
89 89 
 	defer resp.Close()
90 
+	if in != nil && execConfig.Tty {
91 
+		if err := cli.setRawTerminal(); err != nil {
92 
+			return err
93 
+		}
94 
+		defer cli.restoreTerminal(in)
95 
+	}
90 96 
 	errCh = promise.Go(func() error {
91 97 
 		return cli.holdHijackedConnection(execConfig.Tty, in, out, stderr, resp)
92 98 
 	})
api/client/hijack.go
History View file @ a1eb602
... ... 
@@ -2,41 +2,19 @@ package client
2 2
3 3 
 import (
4 4 
 	"io"
5 
-	"os"
6 5
7 6 
 	"github.com/Sirupsen/logrus"
8 7 
 	"github.com/docker/docker/pkg/stdcopy"
9 
-	"github.com/docker/docker/pkg/term"
10 8 
 	"github.com/docker/engine-api/types"
11 9 
 )
12 10
13 
-func (cli *DockerCli) holdHijackedConnection(setRawTerminal bool, inputStream io.ReadCloser, outputStream, errorStream io.Writer, resp types.HijackedResponse) error {
14 
-	var (
15 
-		err      error
16 
-		oldState *term.State
17 
-	)
18 
-	if inputStream != nil && setRawTerminal && cli.isTerminalIn && os.Getenv("NORAW") == "" {
19 
-		oldState, err = term.SetRawTerminal(cli.inFd)
20 
-		if err != nil {
21 
-			return err
22 
-		}
23 
-		defer term.RestoreTerminal(cli.inFd, oldState)
24 
-	}
25 
-
11 
+func (cli *DockerCli) holdHijackedConnection(tty bool, inputStream io.ReadCloser, outputStream, errorStream io.Writer, resp types.HijackedResponse) error {
12 
+	var err error
26 13 
 	receiveStdout := make(chan error, 1)
27 14 
 	if outputStream != nil || errorStream != nil {
28 15 
 		go func() {
29 
-			defer func() {
30 
-				if inputStream != nil {
31 
-					if setRawTerminal && cli.isTerminalIn {
32 
-						term.RestoreTerminal(cli.inFd, oldState)
33 
-					}
34 
-					inputStream.Close()
35 
-				}
36 
-			}()
37 
-
38 16 
 			// When TTY is ON, use regular copy
39 
-			if setRawTerminal && outputStream != nil {
17 
+			if tty && outputStream != nil {
40 18 
 				_, err = io.Copy(outputStream, resp.Reader)
41 19 
 			} else {
42 20 
 				_, err = stdcopy.StdCopy(outputStream, errorStream, resp.Reader)
api/client/run.go
History View file @ a1eb602
... ... 
@@ -207,6 +207,12 @@ func (cli *DockerCli) CmdRun(args ...string) error {
207 207 
 		if err != nil {
208 208 
 			return err
209 209 
 		}
210 
+		if in != nil && config.Tty {
211 
+			if err := cli.setRawTerminal(); err != nil {
212 
+				return err
213 
+			}
214 
+			defer cli.restoreTerminal(in)
215 
+		}
210 216 
 		errCh = promise.Go(func() error {
211 217 
 			return cli.holdHijackedConnection(config.Tty, in, out, stderr, resp)
212 218 
 		})
api/client/start.go
History View file @ a1eb602
... ... 
@@ -96,6 +96,12 @@ func (cli *DockerCli) CmdStart(args ...string) error {
96 96 
 			return err
97 97 
 		}
98 98 
 		defer resp.Close()
99 
+		if in != nil && c.Config.Tty {
100 
+			if err := cli.setRawTerminal(); err != nil {
101 
+				return err
102 
+			}
103 
+			defer cli.restoreTerminal(in)
104 
+		}
99 105
100 106 
 		cErr := promise.Go(func() error {
101 107 
 			return cli.holdHijackedConnection(c.Config.Tty, in, cli.out, cli.err, resp)