1. docker
  2. Commit a88e13f4f95487f469ca5996ffdef865d47fd869
Browse code

client: fix datarace when accessing cli.Version field

Originally I've found this datarace on a project I'm working at. I'm not
able to consistently reproduce this. But by looking at the codebase I
took a chance to fix other 2 possible function that might produce such
data race.

Original stack trace produced when running `go test -race` on GH CI:

```
WARNING: DATA RACE
Write at 0x00c0005dc688 by goroutine 43:
github.com/docker/docker/client.(*Client).negotiateAPIVersionPing()
/home/runner/go/pkg/mod/github.com/docker/docker@v28.2.2+incompatible/client/client.go:389 +0x12f
github.com/docker/docker/client.(*Client).checkVersion()
/home/runner/go/pkg/mod/github.com/docker/docker@v28.2.2+incompatible/client/client.go:298 +0x249
github.com/docker/docker/client.(*Client).getAPIPath()
/home/runner/go/pkg/mod/github.com/docker/docker@v28.2.2+incompatible/client/client.go:307 +0x76
github.com/docker/docker/client.(*Client).sendRequest()
/home/runner/go/pkg/mod/github.com/docker/docker@v28.2.2+incompatible/client/request.go:111 +0x9b
github.com/docker/docker/client.(*Client).get()
/home/runner/go/pkg/mod/github.com/docker/docker@v28.2.2+incompatible/client/request.go:28 +0x736
github.com/docker/docker/client.(*Client).ContainerList()
/home/runner/go/pkg/mod/github.com/docker/docker@v28.2.2+incompatible/client/container_list.go:47 +0x6f0

Previous read at 0x00c0005dc688 by goroutine 42:
github.com/docker/docker/client.(*Client).ContainerList()
/home/runner/go/pkg/mod/github.com/docker/docker@v28.2.2+incompatible/client/container_list.go:39 +0x5ef
```

Co-authored-by: Luca Rinaldi <lucarin@protonmail.com>
Signed-off-by: Alessio Perugini <alessio@perugini.xyz>

Alessio Perugini authored on 2025/07/18 15:36:04
Showing 3 changed files
client/container_exec.go
History View file @ a88e13f
... ... 
@@ -46,6 +46,15 @@ func (cli *Client) ContainerExecCreate(ctx context.Context, containerID string,
46 46
47 47 
 // ContainerExecStart starts an exec process already created in the docker host.
48 48 
 func (cli *Client) ContainerExecStart(ctx context.Context, execID string, config container.ExecStartOptions) error {
49 
+	// Make sure we negotiated (if the client is configured to do so),
50 
+	// as code below contains API-version specific handling of options.
51 
+	//
52 
+	// Normally, version-negotiation (if enabled) would not happen until
53 
+	// the API request is made.
54 
+	if err := cli.checkVersion(ctx); err != nil {
55 
+		return err
56 
+	}
57 
+
49 58 
 	if versions.LessThan(cli.ClientVersion(), "1.42") {
50 59 
 		config.ConsoleSize = nil
51 60 
 	}
client/container_list.go
History View file @ a88e13f
... ... 
@@ -35,6 +35,15 @@ func (cli *Client) ContainerList(ctx context.Context, options container.ListOpti
35 35 
 	}
36 36
37 37 
 	if options.Filters.Len() > 0 {
38 
+		// Make sure we negotiated (if the client is configured to do so),
39 
+		// as code below contains API-version specific handling of options.
40 
+		//
41 
+		// Normally, version-negotiation (if enabled) would not happen until
42 
+		// the API request is made.
43 
+		if err := cli.checkVersion(ctx); err != nil {
44 
+			return nil, err
45 
+		}
46 
+
38 47 
 		//nolint:staticcheck // ignore SA1019 for old code
39 48 
 		filterJSON, err := filters.ToParamWithVersion(cli.version, options.Filters)
40 49 
 		if err != nil {
client/events.go
History View file @ a88e13f
... ... 
@@ -23,6 +23,16 @@ func (cli *Client) Events(ctx context.Context, options events.ListOptions) (<-ch
23 23 
 	go func() {
24 24 
 		defer close(errs)
25 25
26 
+		// Make sure we negotiated (if the client is configured to do so),
27 
+		// as code below contains API-version specific handling of options.
28 
+		//
29 
+		// Normally, version-negotiation (if enabled) would not happen until
30 
+		// the API request is made.
31 
+		if err := cli.checkVersion(ctx); err != nil {
32 
+			close(started)
33 
+			errs <- err
34 
+			return
35 
+		}
26 36 
 		query, err := buildEventsQueryParams(cli.version, options)
27 37 
 		if err != nil {
28 38 
 			close(started)