diff --git a/api/client/attach.go b/api/client/attach.go index e89644d6b0..144d47efb6 100644 --- a/api/client/attach.go +++ b/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 } diff --git a/api/client/exec.go b/api/client/exec.go index 520c3a382e..c96f03a0bd 100644 --- a/api/client/exec.go +++ b/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 { diff --git a/api/client/hijack.go b/api/client/hijack.go index 4c80fe1cd9..38bd6a7afb 100644 --- a/api/client/hijack.go +++ b/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 diff --git a/api/client/run.go b/api/client/run.go index 4e6caa3e1f..ec0a4ea856 100644 --- a/api/client/run.go +++ b/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) } diff --git a/api/client/start.go b/api/client/start.go index 1ff2845f55..0d141da88c 100644 --- a/api/client/start.go +++ b/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 }