client: fix datarace when accessing cli.Version field

[alessio-perugini]

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

Go SDK: Fix data race in `ContainerExecStart`, `ContainerList`, and `Events`.

[vvoland]

Not sure if it's correct solution - perhaps this should be handled by cli.ClientVersion() and then we should change all cli.version usages to use the accessor?

Overall, I'm concerned about having to explicitly call the checkVersion.

[alessio-perugini]

@vvoland I 💯 agree. Before coming up with this PR, I looked at the codebase, and it seems we have plenty of other places where we're doing the exact same thing. That's how I came up with this PR. But it is definitely error-prone to handle this case like that.

For reference, below I'm linking the same usage in a different part of the repo (might be useful for a future refactoring):

• https://github.com/search?q=repo%3Amoby%2Fmoby%20cli.checkVersion(ctx)&type=code:

  • moby/client/image_list.go

    Line 27 in 22bb28c

    if err := cli.checkVersion(ctx); err != nil {

  • moby/client/volume_remove.go

    Line 24 in 22bb28c

    if err := cli.checkVersion(ctx); err != nil {

  • moby/client/container_stop.go

    Line 35 in 22bb28c

    if err := cli.checkVersion(ctx); err != nil {

  • moby/client/network_create.go

    Line 18 in 22bb28c

    if err := cli.checkVersion(ctx); err != nil {

  • moby/client/client.go

    Line 316 in 22bb28c

    _ = cli.checkVersion(ctx)

  • moby/client/container_restart.go

    Line 31 in 22bb28c

    if err := cli.checkVersion(ctx); err != nil {

  • moby/client/container_create.go

    Line 28 in 22bb28c

    if err := cli.checkVersion(ctx); err != nil {

  • moby/client/container_wait.go

    Line 47 in 22bb28c

    if err := cli.checkVersion(ctx); err != nil {

  • moby/client/container_exec.go

    Line 25 in 22bb28c

    if err := cli.checkVersion(ctx); err != nil {

  • moby/client/service_update.go

    Line 28 in 22bb28c

    if err := cli.checkVersion(ctx); err != nil {

  • moby/client/errors.go

    Line 82 in 22bb28c

    if err := cli.checkVersion(ctx); err != nil {

[vvoland]

Oh, good point! I didn't notice we already do this 🙈

We should take a look into it as a follow up.

[vvoland]

LGTM, thanks!

[thaJeztah]

This one made changes to the client module, so needed a re-vendor (as we vendor the client module)