Race condition in dockertest.Network struct, when deploying mutiple containers concurrently using dockertest.RunWithOptions

[cksidharthan]

Preflight checklist

• I could not find a solution in the existing issues, docs, nor discussions.
• I agree to follow this project's Code of Conduct.
• I have read and am following this repository's Contribution Guidelines.
• I have joined the Ory Community Slack.
• I am signed up to the Ory Security Patch Newsletter.

Ory Network Project

https://github.com/ory/dockertest

Describe the bug

When trying to deploy multiple containers via goroutines, there is a race condition since the Network struct is used in the RunWithOptions method as below

for _, network := range opts.Networks {
	network.Network, err = d.Client.NetworkInfo(network.Network.ID)
	if err != nil {
		return nil, err
	}
}

We encountered this because we are deploying multiple different containers to prepare our test suite and therefore to save time we use goroutines and call the RunWithOptions from inside them

Reproducing the bug

To replicate thsi bug, run the below test with -race flag.

func TestNetworkRaceCondition(t *testing.T) {
	network, err := pool.CreateNetwork(fmt.Sprintf("test-network-race-condition-%d", time.Now().Unix()))
	require.NoError(t, err)
	defer network.Close()

	resources := make([]*dockertest.Resource, 10)
	var wg sync.WaitGroup

	for i := 0; i < 10; i++ {
		wg.Add(1)
		// RunWithOptions must be called in parallel to recreate the issue
		go func(i int) {
			defer wg.Done()
			resource, containerErr := pool.RunWithOptions(
				&dockertest.RunOptions{
					Repository: "postgres",
					Tag:        "13.4",
					Networks:   []*dockertest.Network{network},
				},
			)
			require.NoError(t, containerErr)
			resources[i] = resource
		}(i)
	}

	wg.Wait()

	for i := 0; i < 10; i++ {
		resource := resources[i]
		require.NoError(t, pool.Purge(resource))
	}
}

Relevant log output

==================
WARNING: DATA RACE
Write at 0x00c00041b6d0 by goroutine 747:
  github.com/ory/dockertest/v3.(*Pool).RunWithOptions()
      /Users/dev/cksidharthan/dockertest/dockertest.go:502 +0x13f8
  github.com/ory/dockertest/v3_test.TestNetworkRaceCondition.func1()
      /Users/dev/cksidharthan/dockertest/dockertest_test.go:493 +0x198
  github.com/ory/dockertest/v3_test.TestNetworkRaceCondition.gowrap2()
      /Users/dev/cksidharthan/dockertest/dockertest_test.go:502 +0x44

Previous write at 0x00c00041b6d0 by goroutine 749:
  github.com/ory/dockertest/v3.(*Pool).RunWithOptions()
      /Users/dev/cksidharthan/dockertest/dockertest.go:502 +0x13f8
  github.com/ory/dockertest/v3_test.TestNetworkRaceCondition.func1()
      /Users/dev/cksidharthan/dockertest/dockertest_test.go:493 +0x198
  github.com/ory/dockertest/v3_test.TestNetworkRaceCondition.gowrap2()
      /Users/dev/cksidharthan/dockertest/dockertest_test.go:502 +0x44

Goroutine 747 (running) created at:
  github.com/ory/dockertest/v3_test.TestNetworkRaceCondition()
      /Users/dev/cksidharthan/dockertest/dockertest_test.go:491 +0x1e0
  testing.tRunner()
      /opt/homebrew/opt/go/libexec/src/testing/testing.go:1792 +0x180
  testing.(*T).Run.gowrap1()
      /opt/homebrew/opt/go/libexec/src/testing/testing.go:1851 +0x40

Goroutine 749 (finished) created at:
  github.com/ory/dockertest/v3_test.TestNetworkRaceCondition()
      /Users/dev/cksidharthan/dockertest/dockertest_test.go:491 +0x1e0
  testing.tRunner()
      /opt/homebrew/opt/go/libexec/src/testing/testing.go:1792 +0x180
  testing.(*T).Run.gowrap1()
      /opt/homebrew/opt/go/libexec/src/testing/testing.go:1851 +0x40
==================
==================
WARNING: DATA RACE
Read at 0x00c00041b6d0 by goroutine 745:
  github.com/ory/dockertest/v3.(*Pool).RunWithOptions()
      /Users/dev/cksidharthan/dockertest/dockertest.go:502 +0x13ac
  github.com/ory/dockertest/v3_test.TestNetworkRaceCondition.func1()
      /Users/dev/cksidharthan/dockertest/dockertest_test.go:493 +0x198
  github.com/ory/dockertest/v3_test.TestNetworkRaceCondition.gowrap2()
      /Users/dev/cksidharthan/dockertest/dockertest_test.go:502 +0x44

Previous write at 0x00c00041b6d0 by goroutine 744:
  github.com/ory/dockertest/v3.(*Pool).RunWithOptions()
      /Users/dev/cksidharthan/dockertest/dockertest.go:502 +0x13f8
  github.com/ory/dockertest/v3_test.TestNetworkRaceCondition.func1()
      /Users/dev/cksidharthan/dockertest/dockertest_test.go:493 +0x198
  github.com/ory/dockertest/v3_test.TestNetworkRaceCondition.gowrap2()
      /Users/dev/cksidharthan/dockertest/dockertest_test.go:502 +0x44

Goroutine 745 (running) created at:
  github.com/ory/dockertest/v3_test.TestNetworkRaceCondition()
      /Users/dev/cksidharthan/dockertest/dockertest_test.go:491 +0x1e0
  testing.tRunner()
      /opt/homebrew/opt/go/libexec/src/testing/testing.go:1792 +0x180
  testing.(*T).Run.gowrap1()
      /opt/homebrew/opt/go/libexec/src/testing/testing.go:1851 +0x40

Goroutine 744 (finished) created at:
  github.com/ory/dockertest/v3_test.TestNetworkRaceCondition()
      /Users/dev/cksidharthan/dockertest/dockertest_test.go:491 +0x1e0
  testing.tRunner()
      /opt/homebrew/opt/go/libexec/src/testing/testing.go:1792 +0x180
  testing.(*T).Run.gowrap1()
      /opt/homebrew/opt/go/libexec/src/testing/testing.go:1851 +0x40
==================
==================
WARNING: DATA RACE
Read at 0x00c0004041c0 by goroutine 746:
  github.com/ory/dockertest/v3.(*Pool).RunWithOptions()
      /Users/dev/cksidharthan/dockertest/dockertest.go:502 +0x13c4
  github.com/ory/dockertest/v3_test.TestNetworkRaceCondition.func1()
      /Users/dev/cksidharthan/dockertest/dockertest_test.go:493 +0x198
  github.com/ory/dockertest/v3_test.TestNetworkRaceCondition.gowrap2()
      /Users/dev/cksidharthan/dockertest/dockertest_test.go:502 +0x44

Previous write at 0x00c0004041c0 by goroutine 745:
  github.com/ory/dockertest/v3/docker.(*Client).NetworkInfo()
      /Users/dev/cksidharthan/dockertest/docker/network.go:103 +0x128
  github.com/ory/dockertest/v3.(*Pool).RunWithOptions()
      /Users/dev/cksidharthan/dockertest/dockertest.go:502 +0x13e4
  github.com/ory/dockertest/v3_test.TestNetworkRaceCondition.func1()
      /Users/dev/cksidharthan/dockertest/dockertest_test.go:493 +0x198
  github.com/ory/dockertest/v3_test.TestNetworkRaceCondition.gowrap2()
      /Users/dev/cksidharthan/dockertest/dockertest_test.go:502 +0x44

Goroutine 746 (running) created at:
  github.com/ory/dockertest/v3_test.TestNetworkRaceCondition()
      /Users/dev/cksidharthan/dockertest/dockertest_test.go:491 +0x1e0
  testing.tRunner()
      /opt/homebrew/opt/go/libexec/src/testing/testing.go:1792 +0x180
  testing.(*T).Run.gowrap1()
      /opt/homebrew/opt/go/libexec/src/testing/testing.go:1851 +0x40

Goroutine 745 (finished) created at:
  github.com/ory/dockertest/v3_test.TestNetworkRaceCondition()
      /Users/dev/cksidharthan/dockertest/dockertest_test.go:491 +0x1e0
  testing.tRunner()
      /opt/homebrew/opt/go/libexec/src/testing/testing.go:1792 +0x180
  testing.(*T).Run.gowrap1()
      /opt/homebrew/opt/go/libexec/src/testing/testing.go:1851 +0x40
==================
==================
WARNING: DATA RACE
Read at 0x00c0004b0760 by goroutine 750:
  github.com/ory/dockertest/v3.(*Pool).RunWithOptions()
      /Users/dev/cksidharthan/dockertest/dockertest.go:502 +0x13c4
  github.com/ory/dockertest/v3_test.TestNetworkRaceCondition.func1()
      /Users/dev/cksidharthan/dockertest/dockertest_test.go:493 +0x198
  github.com/ory/dockertest/v3_test.TestNetworkRaceCondition.gowrap2()
      /Users/dev/cksidharthan/dockertest/dockertest_test.go:502 +0x44

Previous write at 0x00c0004b0760 by goroutine 747:
  reflect.Value.SetString()
      /opt/homebrew/opt/go/libexec/src/reflect/value.go:2217 +0x80
  encoding/json.(*decodeState).literalStore()
      /opt/homebrew/opt/go/libexec/src/encoding/json/decode.go:957 +0xda8
  encoding/json.(*decodeState).value()
      /opt/homebrew/opt/go/libexec/src/encoding/json/decode.go:389 +0x19c
  encoding/json.(*decodeState).object()
      /opt/homebrew/opt/go/libexec/src/encoding/json/decode.go:762 +0xf64
  encoding/json.(*decodeState).value()
      /opt/homebrew/opt/go/libexec/src/encoding/json/decode.go:375 +0x78
  encoding/json.(*decodeState).unmarshal()
      /opt/homebrew/opt/go/libexec/src/encoding/json/decode.go:178 +0x23c
  encoding/json.(*Decoder).Decode()
      /opt/homebrew/opt/go/libexec/src/encoding/json/stream.go:73 +0x278
  github.com/ory/dockertest/v3/docker.(*Client).NetworkInfo()
      /Users/dev/cksidharthan/dockertest/docker/network.go:104 +0x39c
  github.com/ory/dockertest/v3.(*Pool).RunWithOptions()
      /Users/dev/cksidharthan/dockertest/dockertest.go:502 +0x13e4
  github.com/ory/dockertest/v3_test.TestNetworkRaceCondition.func1()
      /Users/dev/cksidharthan/dockertest/dockertest_test.go:493 +0x198
  github.com/ory/dockertest/v3_test.TestNetworkRaceCondition.gowrap2()
      /Users/dev/cksidharthan/dockertest/dockertest_test.go:502 +0x44

Goroutine 750 (running) created at:
  github.com/ory/dockertest/v3_test.TestNetworkRaceCondition()
      /Users/dev/cksidharthan/dockertest/dockertest_test.go:491 +0x1e0
  testing.tRunner()
      /opt/homebrew/opt/go/libexec/src/testing/testing.go:1792 +0x180
  testing.(*T).Run.gowrap1()
      /opt/homebrew/opt/go/libexec/src/testing/testing.go:1851 +0x40

Goroutine 747 (finished) created at:
  github.com/ory/dockertest/v3_test.TestNetworkRaceCondition()
      /Users/dev/cksidharthan/dockertest/dockertest_test.go:491 +0x1e0
  testing.tRunner()
      /opt/homebrew/opt/go/libexec/src/testing/testing.go:1792 +0x180
  testing.(*T).Run.gowrap1()
      /opt/homebrew/opt/go/libexec/src/testing/testing.go:1851 +0x40
==================
--- FAIL: TestNetworkRaceCondition (1.35s)
    testing.go:1490: race detected during execution of test
FAIL
coverage: 56.7% of statements
FAIL    github.com/ory/dockertest/v3    32.976s
FAIL
make: *** [test] Error 1

Relevant configuration

Version

v3.12.0

On which operating system are you observing this issue?

macOS

In which environment are you deploying?

None

Additional Context

As a workaround for this race condition, we can copy the network to a new variable and use it before calling RunWithOptions within our goroutines. But this PR fixes the issue by adding a RWMutex to the network struct and modifying the code in RunWithOptions as below

for _, network := range opts.Networks {
	network.mu.Lock()
	network.Network, err = d.Client.NetworkInfo(network.Network.ID)
	network.mu.Unlock()
	if err != nil {
		return nil, err
	}
}

I have this fix ready in my fork. if the maintainers give approval, I can raise a Pull request :)

[pellared]

Why have this been closed? We possibly got the same issue in open-telemetry/opentelemetry-ebpf-instrumentation#1232. Should I create a new issue or can this be reopened?

[cksidharthan]

Hi @pellared we can reopen this issue. I'll reopen my PR with the fix for this

[pellared]

Hi @pellared we can reopen this issue

Please do so. I think you can also create a PR.

[cksidharthan]

Hi @pellared we can reopen this issue

Please do so. I think you can also create a PR.

Have created it now :) #647