From abe0f8244bf210e4dcd15ab553f2cdcb034345be Mon Sep 17 00:00:00 2001
From: Viktor Stanchev <vikstrous@users.noreply.github.com>
Date: Fri, 20 Dec 2024 01:18:49 -0500
Subject: [PATCH] fix: avoid double lock in DockerProvider.DaemonHost() (#2900)

* avoid double lock in DockerProvider.DaemonHost()

* cleaner structure

* put comment back

* add regression test

* use require

* test improvements

* better error output

* try to fix rootless mode

* pass on XDG_RUNTIME_DIR

* fix: DaemonHost locking test

Fix the DaemonHost locking test by implementing a way to change the
location of the file the core library tests for.

---------

Co-authored-by: Steven Hartland <stevenmhartland@gmail.com>
---
 docker.go                    | 12 +++++++++++-
 docker_test.go               | 38 ++++++++++++++++++++++++++++++++++++
 internal/core/docker_host.go |  7 ++++++-
 3 files changed, 55 insertions(+), 2 deletions(-)

diff --git a/docker.go b/docker.go
index b10b14b7ff..01b3d3d4d2 100644
--- a/docker.go
+++ b/docker.go
@@ -1495,7 +1495,11 @@ func (p *DockerProvider) daemonHostLocked(ctx context.Context) (string, error) {
 		p.hostCache = daemonURL.Hostname()
 	case "unix", "npipe":
 		if core.InAContainer() {
-			ip, err := p.GetGatewayIP(ctx)
+			defaultNetwork, err := p.ensureDefaultNetworkLocked(ctx)
+			if err != nil {
+				return "", fmt.Errorf("ensure default network: %w", err)
+			}
+			ip, err := p.getGatewayIP(ctx, defaultNetwork)
 			if err != nil {
 				ip, err = core.DefaultGatewayIP()
 				if err != nil {
@@ -1595,7 +1599,10 @@ func (p *DockerProvider) GetGatewayIP(ctx context.Context) (string, error) {
 	if err != nil {
 		return "", fmt.Errorf("ensure default network: %w", err)
 	}
+	return p.getGatewayIP(ctx, defaultNetwork)
+}
 
+func (p *DockerProvider) getGatewayIP(ctx context.Context, defaultNetwork string) (string, error) {
 	nw, err := p.GetNetwork(ctx, NetworkRequest{Name: defaultNetwork})
 	if err != nil {
 		return "", err
@@ -1621,7 +1628,10 @@ func (p *DockerProvider) GetGatewayIP(ctx context.Context) (string, error) {
 func (p *DockerProvider) ensureDefaultNetwork(ctx context.Context) (string, error) {
 	p.mtx.Lock()
 	defer p.mtx.Unlock()
+	return p.ensureDefaultNetworkLocked(ctx)
+}
 
+func (p *DockerProvider) ensureDefaultNetworkLocked(ctx context.Context) (string, error) {
 	if p.defaultNetwork != "" {
 		// Already set.
 		return p.defaultNetwork, nil
diff --git a/docker_test.go b/docker_test.go
index eb92e15060..8fcd60c558 100644
--- a/docker_test.go
+++ b/docker_test.go
@@ -25,6 +25,7 @@ import (
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 
+	"github.com/testcontainers/testcontainers-go/internal/core"
 	"github.com/testcontainers/testcontainers-go/wait"
 )
 
@@ -35,6 +36,7 @@ const (
 	nginxAlpineImage  = "nginx:alpine"
 	nginxDefaultPort  = "80/tcp"
 	nginxHighPort     = "8080/tcp"
+	golangImage       = "golang"
 	daemonMaxVersion  = "1.41"
 )
 
@@ -2156,3 +2158,39 @@ func TestCustomPrefixTrailingSlashIsProperlyRemovedIfPresent(t *testing.T) {
 	dockerContainer := c.(*DockerContainer)
 	require.Equal(t, fmt.Sprintf("%s%s", hubPrefixWithTrailingSlash, dockerImage), dockerContainer.Image)
 }
+
+// TODO: remove this skip check when context rework is merged alongside [core.DockerEnvFile] removal.
+func Test_Provider_DaemonHost_Issue2897(t *testing.T) {
+	ctx := context.Background()
+	provider, err := NewDockerProvider()
+	require.NoError(t, err)
+	t.Cleanup(func() {
+		require.NoError(t, provider.Close())
+	})
+
+	orig := core.DockerEnvFile
+	core.DockerEnvFile = filepath.Join(t.TempDir(), ".dockerenv")
+	t.Cleanup(func() {
+		core.DockerEnvFile = orig
+	})
+
+	f, err := os.Create(core.DockerEnvFile)
+	require.NoError(t, err)
+	require.NoError(t, f.Close())
+	t.Cleanup(func() {
+		require.NoError(t, os.Remove(f.Name()))
+	})
+
+	errCh := make(chan error, 1)
+	go func() {
+		_, err := provider.DaemonHost(ctx)
+		errCh <- err
+	}()
+
+	select {
+	case <-time.After(1 * time.Second):
+		t.Fatal("timeout waiting for DaemonHost")
+	case err := <-errCh:
+		require.NoError(t, err)
+	}
+}
diff --git a/internal/core/docker_host.go b/internal/core/docker_host.go
index 3088a3742b..765626da57 100644
--- a/internal/core/docker_host.go
+++ b/internal/core/docker_host.go
@@ -309,10 +309,15 @@ func testcontainersHostFromProperties(ctx context.Context) (string, error) {
 	return "", ErrTestcontainersHostNotSetInProperties
 }
 
+// DockerEnvFile is the file that is created when running inside a container.
+// It's a variable to allow testing.
+// TODO: Remove this once context rework is done, which eliminates need for the default network creation.
+var DockerEnvFile = "/.dockerenv"
+
 // InAContainer returns true if the code is running inside a container
 // See https://github.com/docker/docker/blob/a9fa38b1edf30b23cae3eade0be48b3d4b1de14b/daemon/initlayer/setup_unix.go#L25
 func InAContainer() bool {
-	return inAContainer("/.dockerenv")
+	return inAContainer(DockerEnvFile)
 }
 
 func inAContainer(path string) bool {