From c27751fcfe5ded9b3e1e6afe0f1c298d058b72c9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pawe=C5=82=20Gronowski?= Date: Mon, 24 Mar 2025 15:27:28 +0100 Subject: [PATCH] container/run: Fix stdout/err truncation after container exit MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Fix a regression introduced by 30c4637f03ecb8855fe6e47f5374c89d50222577 which made the `docker run` command produce potentially truncated stdout/stderr output. Previous implementation stopped the content streaming as soon as the container exited which would potentially truncate a long outputs. This change fixes the issue by only canceling the IO stream immediately if neither stdout nor stderr is attached. Signed-off-by: Paweł Gronowski --- cli/command/container/run.go | 14 ++++++--- e2e/container/run_test.go | 56 ++++++++++++++++++++++++++++++++++++ 2 files changed, 66 insertions(+), 4 deletions(-) diff --git a/cli/command/container/run.go b/cli/command/container/run.go index 5c541a0dde..702669e19b 100644 --- a/cli/command/container/run.go +++ b/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} } diff --git a/e2e/container/run_test.go b/e2e/container/run_test.go index ec56d03b2b..d279f2521b 100644 --- a/e2e/container/run_test.go +++ b/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)) +}