Skip to content

Commit b628457

Browse files
dean-islandDean Balandin
andauthored
fix: read exec exit code only after draining the output stream (#3766)
A created-but-not-yet-started exec is reported by the daemon as {Running:false, ExitCode:null}, which the Docker Go SDK flattens to exit code 0. Inspecting immediately after attach could land in the pre-start window and return 0 for a command still running. Drain the hijacked stream to EOF before inspecting: the daemon closes the stream only once the exec process has exited, making the subsequent Running:false a real terminal state. The output is buffered internally so callers still read it from the returned reader unchanged. On context cancellation Exec returns ctx.Err() promptly; the deferred hijack.Close unblocks the reader goroutine. Also closes the hijacked response, which was previously leaked. Co-authored-by: Dean Balandin <dbalandin@island.io>
1 parent abc841d commit b628457

2 files changed

Lines changed: 54 additions & 2 deletions

File tree

docker.go

Lines changed: 31 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ package testcontainers
33
import (
44
"archive/tar"
55
"bufio"
6+
"bytes"
67
"context"
78
"encoding/binary"
89
"errors"
@@ -586,15 +587,39 @@ func (c *DockerContainer) Exec(ctx context.Context, cmd []string, options ...tce
586587
if err != nil {
587588
return 0, nil, fmt.Errorf("container exec attach: %w", err)
588589
}
590+
defer hijack.Close()
589591

590-
processOptions.Reader = hijack.Reader
592+
// A not-yet-started exec inspects as {Running:false, ExitCode:null->0}, which
593+
// is indistinguishable from "exited 0". The daemon closes the stream only once
594+
// the process ends, so drain to EOF before inspecting; buffer it so callers
595+
// can still read the output from the returned reader.
596+
var buf bytes.Buffer
597+
drained := make(chan error, 1) // buffered so the goroutine can finish after we return on ctx cancel
598+
go func() {
599+
_, copyErr := io.Copy(&buf, hijack.Reader)
600+
drained <- copyErr
601+
}()
602+
603+
select {
604+
case copyErr := <-drained:
605+
if copyErr != nil {
606+
return 0, nil, fmt.Errorf("container exec read: %w", copyErr)
607+
}
608+
case <-ctx.Done():
609+
return 0, nil, ctx.Err()
610+
}
611+
612+
processOptions.Reader = bytes.NewReader(buf.Bytes())
591613

592614
// second loop to process the multiplexed option, as now we have a reader
593615
// from the created exec response.
594616
for _, o := range options {
595617
o.Apply(processOptions)
596618
}
597619

620+
// The daemon marks the exec stopped asynchronously after closing the stream,
621+
// so a single immediate inspect can still observe Running:true; poll until it
622+
// reports terminal.
598623
var exitCode int
599624
for {
600625
execResp, err := cli.ExecInspect(ctx, response.ID, client.ExecInspectOptions{})
@@ -607,7 +632,11 @@ func (c *DockerContainer) Exec(ctx context.Context, cmd []string, options ...tce
607632
break
608633
}
609634

610-
time.Sleep(100 * time.Millisecond)
635+
select {
636+
case <-ctx.Done():
637+
return 0, nil, ctx.Err()
638+
case <-time.After(100 * time.Millisecond):
639+
}
611640
}
612641

613642
return exitCode, processOptions.Reader, nil

docker_exec_test.go

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -71,6 +71,29 @@ func TestExecWithOptions(t *testing.T) {
7171
}
7272
}
7373

74+
func TestExecReturnsNonZeroExitCodeAndOutput(t *testing.T) {
75+
// Regression for exit codes read before the output stream drained: a
76+
// created-but-not-yet-started exec reports {Running:false, ExitCode:null},
77+
// which the SDK flattens to 0, so an inspect landing in the pre-start window
78+
// returned 0 for a command that ultimately failed. Draining the stream to EOF
79+
// first makes the subsequent inspect a real terminal state. The race needs
80+
// daemon load to reproduce (~1 hit per few thousand concurrent execs); this
81+
// asserts the correct code and full output for a short-lived failing command.
82+
ctx := context.Background()
83+
84+
ctr, err := Run(ctx, nginxAlpineImage)
85+
CleanupContainer(t, ctr)
86+
require.NoError(t, err)
87+
88+
code, reader, err := ctr.Exec(ctx, []string{"sh", "-c", "echo MARKER; exit 7"}, tcexec.Multiplexed())
89+
require.NoError(t, err)
90+
require.Equal(t, 7, code)
91+
92+
b, err := io.ReadAll(reader)
93+
require.NoError(t, err)
94+
require.Contains(t, string(b), "MARKER")
95+
}
96+
7497
func TestExecWithMultiplexedResponse(t *testing.T) {
7598
ctx := context.Background()
7699

0 commit comments

Comments
 (0)