From 88e5e4cfb4b617b60983c0e05f83f5957fbd371c Mon Sep 17 00:00:00 2001
From: Gabriel Tomitsuka <gabriel@tomitsuka.com>
Date: Thu, 13 Jun 2024 19:37:11 +0000
Subject: [PATCH] Prevent data race during version negotiation

Signed-off-by: Gabriel Tomitsuka <gabriel@tomitsuka.com>
---
 client/client.go | 28 +++++++++++++++++++++++++---
 1 file changed, 25 insertions(+), 3 deletions(-)

diff --git a/client/client.go b/client/client.go
index f2eeb6c570..60d91bc65b 100644
--- a/client/client.go
+++ b/client/client.go
@@ -49,6 +49,8 @@ import (
 	"net/url"
 	"path"
 	"strings"
+	"sync"
+	"sync/atomic"
 	"time"
 
 	"github.com/docker/docker/api"
@@ -131,7 +133,10 @@ type Client struct {
 	negotiateVersion bool
 
 	// negotiated indicates that API version negotiation took place
-	negotiated bool
+	negotiated atomic.Bool
+
+	// negotiateLock is used to single-flight the version negotiation process
+	negotiateLock sync.Mutex
 
 	tp trace.TracerProvider
 
@@ -266,7 +271,16 @@ func (cli *Client) Close() error {
 // be negotiated when making the actual requests, and for which cases
 // we cannot do the negotiation lazily.
 func (cli *Client) checkVersion(ctx context.Context) error {
-	if !cli.manualOverride && cli.negotiateVersion && !cli.negotiated {
+	if !cli.manualOverride && cli.negotiateVersion && !cli.negotiated.Load() {
+		// Ensure exclusive write access to version and negotiated fields
+		cli.negotiateLock.Lock()
+		defer cli.negotiateLock.Unlock()
+
+		// May have been set during last execution of critical zone
+		if cli.negotiated.Load() {
+			return nil
+		}
+
 		ping, err := cli.Ping(ctx)
 		if err != nil {
 			return err
@@ -312,6 +326,10 @@ func (cli *Client) ClientVersion() string {
 // added (1.24).
 func (cli *Client) NegotiateAPIVersion(ctx context.Context) {
 	if !cli.manualOverride {
+		// Avoid concurrent modification of version-related fields
+		cli.negotiateLock.Lock()
+		defer cli.negotiateLock.Unlock()
+
 		ping, err := cli.Ping(ctx)
 		if err != nil {
 			// FIXME(thaJeztah): Ping returns an error when failing to connect to the API; we should not swallow the error here, and instead returning it.
@@ -336,6 +354,10 @@ func (cli *Client) NegotiateAPIVersion(ctx context.Context) {
 // added (1.24).
 func (cli *Client) NegotiateAPIVersionPing(pingResponse types.Ping) {
 	if !cli.manualOverride {
+		// Avoid concurrent modification of version-related fields
+		cli.negotiateLock.Lock()
+		defer cli.negotiateLock.Unlock()
+
 		cli.negotiateAPIVersionPing(pingResponse)
 	}
 }
@@ -361,7 +383,7 @@ func (cli *Client) negotiateAPIVersionPing(pingResponse types.Ping) {
 	// Store the results, so that automatic API version negotiation (if enabled)
 	// won't be performed on the next request.
 	if cli.negotiateVersion {
-		cli.negotiated = true
+		cli.negotiated.Store(true)
 	}
 }