diff --git a/commands.go b/commands.go index 7e62596570..29508ce0a3 100644 --- a/commands.go +++ b/commands.go @@ -803,6 +803,8 @@ func (srv *Server) CmdAttach(stdin io.ReadCloser, stdout rcli.DockerConn, args . if container.Config.Tty { stdout.SetOptionRawTerminal() + // Flush the options to make sure the client sets the raw mode + stdout.Write([]byte{}) } return <-container.Attach(stdin, nil, stdout, stdout) } @@ -888,8 +890,11 @@ func (srv *Server) CmdRun(stdin io.ReadCloser, stdout rcli.DockerConn, args ...s fmt.Fprintln(stdout, "Error: Command not specified") return fmt.Errorf("Command not specified") } + if config.Tty { stdout.SetOptionRawTerminal() + // Flush the options to make sure the client sets the raw mode + stdout.Flush() } // Create new container diff --git a/commands_test.go b/commands_test.go index 6c9dc70d5e..4592ea77ac 100644 --- a/commands_test.go +++ b/commands_test.go @@ -191,6 +191,54 @@ func TestRunDisconnect(t *testing.T) { }) } +// Expected behaviour: the process dies when the client disconnects +func TestRunDisconnectTty(t *testing.T) { + runtime, err := newTestRuntime() + if err != nil { + t.Fatal(err) + } + defer nuke(runtime) + + srv := &Server{runtime: runtime} + + stdin, stdinPipe := io.Pipe() + stdout, stdoutPipe := io.Pipe() + c1 := make(chan struct{}) + go func() { + // We're simulating a disconnect so the return value doesn't matter. What matters is the + // fact that CmdRun returns. + srv.CmdRun(stdin, rcli.NewDockerLocalConn(stdoutPipe), "-i", "-t", GetTestImage(runtime).Id, "/bin/cat") + close(c1) + }() + + setTimeout(t, "Read/Write assertion timed out", 2*time.Second, func() { + if err := assertPipe("hello\n", "hello", stdout, stdinPipe, 15); err != nil { + t.Fatal(err) + } + }) + + // Close pipes (simulate disconnect) + if err := closeWrap(stdin, stdinPipe, stdout, stdoutPipe); err != nil { + t.Fatal(err) + } + + // as the pipes are close, we expect the process to die, + // therefore CmdRun to unblock. Wait for CmdRun + setTimeout(t, "Waiting for CmdRun timed out", 2*time.Second, func() { + <-c1 + }) + + // Client disconnect after run -i should cause stdin to be closed, which should + // cause /bin/cat to exit. + setTimeout(t, "Waiting for /bin/cat to exit timed out", 2*time.Second, func() { + container := runtime.List()[0] + container.Wait() + if container.State.Running { + t.Fatalf("/bin/cat is still running after closing stdin") + } + }) +} + // TestAttachStdin checks attaching to stdin without stdout and stderr. // 'docker run -i -a stdin' should sends the client's stdin to the command, // then detach from it and print the container id. diff --git a/container.go b/container.go index 76e9f5a2d7..6b3913522c 100644 --- a/container.go +++ b/container.go @@ -40,9 +40,7 @@ type Container struct { stdin io.ReadCloser stdinPipe io.WriteCloser - ptyStdinMaster io.Closer - ptyStdoutMaster io.Closer - ptyStderrMaster io.Closer + ptyMaster io.Closer runtime *Runtime } @@ -180,63 +178,37 @@ func (container *Container) generateLXCConfig() error { } func (container *Container) startPty() error { - stdoutMaster, stdoutSlave, err := pty.Open() + ptyMaster, ptySlave, err := pty.Open() if err != nil { return err } - container.ptyStdoutMaster = stdoutMaster - container.cmd.Stdout = stdoutSlave - - stderrMaster, stderrSlave, err := pty.Open() - if err != nil { - return err - } - container.ptyStderrMaster = stderrMaster - container.cmd.Stderr = stderrSlave + container.ptyMaster = ptyMaster + container.cmd.Stdout = ptySlave + container.cmd.Stderr = ptySlave // Copy the PTYs to our broadcasters go func() { defer container.stdout.CloseWriters() Debugf("[startPty] Begin of stdout pipe") - io.Copy(container.stdout, stdoutMaster) + io.Copy(container.stdout, ptyMaster) Debugf("[startPty] End of stdout pipe") }() - go func() { - defer container.stderr.CloseWriters() - Debugf("[startPty] Begin of stderr pipe") - io.Copy(container.stderr, stderrMaster) - Debugf("[startPty] End of stderr pipe") - }() - // stdin - var stdinSlave io.ReadCloser if container.Config.OpenStdin { - var stdinMaster io.WriteCloser - stdinMaster, stdinSlave, err = pty.Open() - if err != nil { - return err - } - container.ptyStdinMaster = stdinMaster - container.cmd.Stdin = stdinSlave - // FIXME: The following appears to be broken. - // "cannot set terminal process group (-1): Inappropriate ioctl for device" - // container.cmd.SysProcAttr = &syscall.SysProcAttr{Setctty: true, Setsid: true} + container.cmd.Stdin = ptySlave + container.cmd.SysProcAttr = &syscall.SysProcAttr{Setctty: true, Setsid: true} go func() { defer container.stdin.Close() Debugf("[startPty] Begin of stdin pipe") - io.Copy(stdinMaster, container.stdin) + io.Copy(ptyMaster, container.stdin) Debugf("[startPty] End of stdin pipe") }() } if err := container.cmd.Start(); err != nil { return err } - stdoutSlave.Close() - stderrSlave.Close() - if stdinSlave != nil { - stdinSlave.Close() - } + ptySlave.Close() return nil } @@ -279,6 +251,9 @@ func (container *Container) Attach(stdin io.ReadCloser, stdinCloser io.Closer, s defer cStderr.Close() } if container.Config.StdinOnce { + if container.Config.Tty { + defer container.Kill() + } defer cStdin.Close() } _, err := io.Copy(cStdin, stdin) @@ -530,19 +505,9 @@ func (container *Container) monitor() { Debugf("%s: Error close stderr: %s", container.Id, err) } - if container.ptyStdinMaster != nil { - if err := container.ptyStdinMaster.Close(); err != nil { - Debugf("%s: Error close pty stdin master: %s", container.Id, err) - } - } - if container.ptyStdoutMaster != nil { - if err := container.ptyStdoutMaster.Close(); err != nil { - Debugf("%s: Error close pty stdout master: %s", container.Id, err) - } - } - if container.ptyStderrMaster != nil { - if err := container.ptyStderrMaster.Close(); err != nil { - Debugf("%s: Error close pty stderr master: %s", container.Id, err) + if container.ptyMaster != nil { + if err := container.ptyMaster.Close(); err != nil { + Debugf("%s: Error closing Pty master: %s", container.Id, err) } } diff --git a/rcli/tcp.go b/rcli/tcp.go index 6fbf2abd09..8c990ed82f 100644 --- a/rcli/tcp.go +++ b/rcli/tcp.go @@ -92,6 +92,11 @@ func (c *DockerTCPConn) Write(b []byte) (int, error) { return n + optionsLen, err } +func (c *DockerTCPConn) Flush() error { + _, err := c.conn.Write([]byte{}) + return err +} + func (c *DockerTCPConn) Close() error { return c.conn.Close() } func (c *DockerTCPConn) CloseWrite() error { return c.conn.CloseWrite() } diff --git a/rcli/types.go b/rcli/types.go index 791736a79c..38f4a8c008 100644 --- a/rcli/types.go +++ b/rcli/types.go @@ -29,6 +29,7 @@ type DockerConn interface { CloseRead() error GetOptions() *DockerConnOptions SetOptionRawTerminal() + Flush() error } type DockerLocalConn struct { @@ -56,6 +57,8 @@ func (c *DockerLocalConn) Close() error { return c.writer.Close() } +func (c *DockerLocalConn) Flush() error { return nil } + func (c *DockerLocalConn) CloseWrite() error { return nil } func (c *DockerLocalConn) CloseRead() error { return nil }