Merge pull request #5957 from vvoland/stdout-trunc

container/run: Fix stdout/err truncation after container exit

Author: vvoland

cli/command/container/run.go

@@ -238,10 +238,16 @@ func runContainer(ctx context.Context, dockerCli command.Cli, runOpts *runOption
 			return cli.StatusError{StatusCode: status}
 		}
 	case status := <-statusChan:
-		// notify hijackedIOStreamer that we're exiting and wait
-		// so that the terminal can be restored.
-		cancelFun()
-		<-errCh
+		// If container exits, output stream processing may not be finished yet,
+		// we need to keep the streamer running until all output is read.
+		// However, if stdout or stderr is not attached, we can just exit.
+		if !config.AttachStdout && !config.AttachStderr {
+			// Notify hijackedIOStreamer that we're exiting and wait
+			// so that the terminal can be restored.
+			cancelFun()
+		}
+		<-errCh // Drain channel but don't care about result
+
 		if status != 0 {
 			return cli.StatusError{StatusCode: status}
 		}

cli/command/container/run_test.go

@@ -147,6 +147,7 @@ func TestRunAttachTermination(t *testing.T) {
 		_ = p.Close()
 	}()
 
+	var conn net.Conn
 	killCh := make(chan struct{})
 	attachCh := make(chan struct{})
 	fakeCLI := test.NewFakeCli(&fakeClient{
@@ -163,6 +164,7 @@ func TestRunAttachTermination(t *testing.T) {
 		},
 		containerAttachFunc: func(ctx context.Context, containerID string, options container.AttachOptions) (types.HijackedResponse, error) {
 			server, client := net.Pipe()
+			conn = server
 			t.Cleanup(func() {
 				_ = server.Close()
 			})
@@ -202,6 +204,7 @@ func TestRunAttachTermination(t *testing.T) {
 	}
 
 	assert.NilError(t, syscall.Kill(syscall.Getpid(), syscall.SIGTERM))
+	conn.Close()
 
 	select {
 	case <-killCh:

e2e/container/run_test.go

@@ -3,6 +3,8 @@ package container
 import (
 	"bytes"
 	"fmt"
+	"io"
+	"math/rand"
 	"os/exec"
 	"strings"
 	"syscall"
@@ -280,3 +282,57 @@ func TestProcessTermination(t *testing.T) {
 		ExitCode: 0,
 	})
 }
+
+// Adapted from https://github.com/docker/for-mac/issues/7632#issue-2932169772
+// Thanks [@almet](https://github.com/almet)!
+func TestRunReadAfterContainerExit(t *testing.T) {
+	skip.If(t, environment.RemoteDaemon())
+
+	r := rand.New(rand.NewSource(0x123456))
+
+	const size = 18933764
+	cmd := exec.Command("docker", "run",
+		"--rm", "-i",
+		"alpine",
+		"sh", "-c", "cat -",
+	)
+
+	cmd.Stdin = io.LimitReader(r, size)
+
+	var stderr bytes.Buffer
+	cmd.Stderr = &stderr
+
+	stdout, err := cmd.StdoutPipe()
+	assert.NilError(t, err)
+
+	err = cmd.Start()
+	assert.NilError(t, err)
+
+	buffer := make([]byte, 1000)
+	counter := 0
+	totalRead := 0
+
+	for {
+		n, err := stdout.Read(buffer)
+		if n > 0 {
+			totalRead += n
+		}
+
+		// Wait 0.1s every megabyte (approx.)
+		if counter%1000 == 0 {
+			time.Sleep(100 * time.Millisecond)
+		}
+
+		if err != nil || n == 0 {
+			break
+		}
+
+		counter++
+	}
+
+	err = cmd.Wait()
+	t.Logf("Error: %v", err)
+	t.Logf("Stderr: %s", stderr.String())
+	assert.Check(t, err == nil)
+	assert.Check(t, is.Equal(totalRead, size))
+}