Merge remote-tracking branch 'origin/pty_fix-1'

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

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.

container.go

@@ -40,9 +40,7 @@ type Container struct {
 	stdin       io.ReadCloser
 	stdinPipe   io.WriteCloser
 
-	ptyStdinMaster  io.Closer
+	ptyMaster io.Closer
-	ptyStdoutMaster io.Closer
-	ptyStderrMaster 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.ptyMaster = ptyMaster
-	container.cmd.Stdout = stdoutSlave
+	container.cmd.Stdout = ptySlave
-
+	container.cmd.Stderr = ptySlave
-	stderrMaster, stderrSlave, err := pty.Open()
-	if err != nil {
-		return err
-	}
-	container.ptyStderrMaster = stderrMaster
-	container.cmd.Stderr = stderrSlave
 
 	// 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
+		container.cmd.Stdin = ptySlave
-		stdinMaster, stdinSlave, err = pty.Open()
+		container.cmd.SysProcAttr = &syscall.SysProcAttr{Setctty: true, Setsid: true}
-		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}
 		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()
+	ptySlave.Close()
-	stderrSlave.Close()
-	if stdinSlave != nil {
-		stdinSlave.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 container.ptyMaster != nil {
-		if err := container.ptyStdinMaster.Close(); err != nil {
+		if err := container.ptyMaster.Close(); err != nil {
-			Debugf("%s: Error close pty stdin master: %s", container.Id, err)
+			Debugf("%s: Error closing Pty 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)
 		}
 	}
 

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() }

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 }