From 89e3d8cbe4d03d884bb5c1bd87cd6573d2ce1c0c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=EC=8B=AC=ED=98=84=EB=AF=BC?= Date: Tue, 25 Aug 2026 23:55:26 +0900 Subject: [PATCH] Fix check-then-act race in container reuse (withReuse(true)) Adds a same-JVM lock keyed by the reuse hash around the existing check-then-create block in GenericContainer.tryStart(), mirroring the synchronized + create-once idiom already used in Network.getId(). For cross-process races, uses Docker's own container-name uniqueness as an arbiter: the create call sets a deterministic name derived from the hash, and a name conflict triggers a bounded retry/lookup instead of failing (verifying the found container's reuse label, waiting out an in-progress creation, cleaning up a dead one). Fixes #11979 --- .../containers/GenericContainer.java | 178 +++++++++++- .../ReusabilityNameConflictTest.java | 256 ++++++++++++++++++ .../ReusabilityRaceConditionTest.java | 186 +++++++++++++ 3 files changed, 609 insertions(+), 11 deletions(-) create mode 100644 core/src/test/java/org/testcontainers/containers/ReusabilityNameConflictTest.java create mode 100644 core/src/test/java/org/testcontainers/containers/ReusabilityRaceConditionTest.java diff --git a/core/src/main/java/org/testcontainers/containers/GenericContainer.java b/core/src/main/java/org/testcontainers/containers/GenericContainer.java index 4d3778c63d1..99f2ac48001 100644 --- a/core/src/main/java/org/testcontainers/containers/GenericContainer.java +++ b/core/src/main/java/org/testcontainers/containers/GenericContainer.java @@ -6,6 +6,7 @@ import com.github.dockerjava.api.DockerClient; import com.github.dockerjava.api.command.CreateContainerCmd; import com.github.dockerjava.api.command.InspectContainerResponse; +import com.github.dockerjava.api.exception.ConflictException; import com.github.dockerjava.api.exception.NotFoundException; import com.github.dockerjava.api.model.Bind; import com.github.dockerjava.api.model.ContainerNetwork; @@ -69,6 +70,7 @@ import java.nio.file.Paths; import java.time.Duration; import java.time.Instant; +import java.util.AbstractMap; import java.util.ArrayList; import java.util.Arrays; import java.util.Collections; @@ -85,6 +87,7 @@ import java.util.ServiceLoader; import java.util.Set; import java.util.UUID; +import java.util.concurrent.ConcurrentHashMap; import java.util.concurrent.ExecutionException; import java.util.concurrent.Future; import java.util.concurrent.TimeUnit; @@ -112,6 +115,22 @@ public class GenericContainer> static final String COPIED_FILES_HASH_LABEL = "org.testcontainers.copied_files.hash"; + /** + * Deterministic name given to a container created for reuse, derived from its reuse hash. + * Docker enforces container-name uniqueness across every process/host sharing a daemon, so + * attempting to create a container under this name is used as a cross-process arbiter: only + * one JVM/process can ever win the "create" call for a given hash, see {@link #tryStart()}. + */ + static final String REUSE_CONTAINER_NAME_PREFIX = "testcontainers-reuse-"; + + /** + * JVM-local lock, keyed by reuse hash, guarding the "check reuse, else create" sequence in + * {@link #tryStart()} against same-JVM races (see "// TODO locking" history on + * {@link #findContainerForReuse(String)}). This does NOT protect against cross-process races. + * Those are handled by the Docker container-name uniqueness arbiter, see {@link #REUSE_CONTAINER_NAME_PREFIX}. + */ + private static final ConcurrentHashMap REUSE_LOCKS = new ConcurrentHashMap<>(); + /* * Default settings */ @@ -391,15 +410,31 @@ private void tryStart() { String hash = hash(createCommand); - containerId = findContainerForReuse(hash).orElse(null); - - if (containerId != null) { - logger().info("Reusing container with ID: {} and hash: {}", containerId, hash); - reused = true; - } else { - logger().debug("Can't find a reusable running container with hash: {}", hash); - - createCommand.getLabels().put(HASH_LABEL, hash); + // Layer 1: JVM-local lock, so two threads in this process can't both miss the + // reuse lookup below and both fall through to creating a container. + Object reuseLock = REUSE_LOCKS.computeIfAbsent(hash, h -> new Object()); + synchronized (reuseLock) { + containerId = findContainerForReuse(hash).orElse(null); + + if (containerId != null) { + logger().info("Reusing container with ID: {} and hash: {}", containerId, hash); + reused = true; + } else { + logger().debug("Can't find a reusable running container with hash: {}", hash); + + createCommand.getLabels().put(HASH_LABEL, hash); + + // Layer 2: the JVM-local lock above only protects this process. Two + // separate processes racing on an identical config hash would both + // still see "not found" here. Use Docker's own container-name + // uniqueness as a cross-process arbiter for the actual create call. + AbstractMap.SimpleEntry createdOrJoined = createOrJoinReusableContainer( + createCommand, + hash + ); + containerId = createdOrJoined.getKey(); + reused = createdOrJoined.getValue(); + } } reusable = true; } else { @@ -421,9 +456,11 @@ private void tryStart() { createCommand = ResourceReaper.instance().register(this, createCommand); } - if (!reused) { + if (!reused && !reusable) { containerId = createCommand.exec().getId(); + } + if (!reused) { // TODO use single "copy" invocation (and calculate an hash of the resulting tar archive) copyToFileContainerPathMap.forEach(this::copyFileToContainer); @@ -587,7 +624,9 @@ final String hash(CreateContainerCmd createCommand) { @VisibleForTesting Optional findContainerForReuse(String hash) { - // TODO locking + // Callers must hold the per-hash lock in REUSE_LOCKS around this call (see tryStart()) to + // avoid a same-JVM check-then-act race; cross-process races are handled separately by + // createOrJoinReusableContainer's name-based arbiter. return dockerClient .listContainersCmd() .withLabelFilter(ImmutableMap.of(HASH_LABEL, hash)) @@ -599,6 +638,123 @@ Optional findContainerForReuse(String hash) { .map(it -> it.getId()); } + /** + * Creates the container for reuse, using a deterministic name derived from the reuse hash as + * a cross-process arbiter: Docker enforces container-name uniqueness daemon-wide, so at most + * one process can ever win the "create" call for a given name/hash, even across separate JVMs + * that all missed the {@link #findContainerForReuse(String)} lookup at the same time. + * + * @return the container id, and whether it was reused (joined a container created by a + * concurrent winner) rather than freshly created by this call. + */ + private AbstractMap.SimpleEntry createOrJoinReusableContainer( + CreateContainerCmd createCommand, + String hash + ) { + String name = REUSE_CONTAINER_NAME_PREFIX + hash; + createCommand.withName(name); + + while (true) { + try { + String newId = createCommand.exec().getId(); + return new AbstractMap.SimpleEntry<>(newId, false); + } catch (ConflictException nameConflict) { + Optional joinedId = resolveConflictingReusableContainer(name, hash); + if (joinedId.isPresent()) { + return new AbstractMap.SimpleEntry<>(joinedId.get(), true); + } + // Name is confirmed free again (the conflicting container vanished, or we just + // cleaned up a dead one). Retry the create call above. + } + } + } + + /** + * Waits out a name conflict on an in-progress concurrent creation, or resolves it directly, + * without re-attempting {@code createCommand.exec()} on every poll. That call is guaranteed + * to fail again as long as the name is still taken, so polling here re-inspects the existing + * container by name instead. + * + * @return the running container's id if one was found and is joinable; empty once the name + * is confirmed free again, meaning the caller should retry the create call. + */ + private Optional resolveConflictingReusableContainer(String name, String hash) { + // Matches AbstractWaitStrategy's default startup timeout. A concurrent winner that + // hasn't finished starting within that budget is treated the same way a normal container + // start that overruns its timeout would be: an error, not an indefinite wait. + Instant deadline = Instant.now().plus(Duration.ofSeconds(60)); + long backoffMillis = 100; + + while (true) { + InspectContainerResponse existing; + try { + existing = dockerClient.inspectContainerCmd(name).exec(); + } catch (NotFoundException goneAlready) { + // The conflicting container vanished since our create attempt (e.g. its own + // creator's process crashed, or Ryuk reaped it). Nothing to join. + return Optional.empty(); + } + + Map existingLabels = existing.getConfig().getLabels(); + String existingHash = existingLabels != null ? existingLabels.get(HASH_LABEL) : null; + if (!hash.equals(existingHash)) { + // A real name clash with something that isn't one of our reuse containers. + // Do NOT blindly reuse it, that could hand back an unrelated container. + throw new IllegalStateException( + "Container name \"" + + name + + "\" is already in use by a container that is not a Testcontainers reuse " + + "container for this configuration (expected label " + + HASH_LABEL + + "=" + + hash + + ", found \"" + + existingHash + + "\")" + ); + } + + String status = existing.getState().getStatus(); + if ("running".equalsIgnoreCase(status)) { + return Optional.of(existing.getId()); + } + + if ("exited".equalsIgnoreCase(status) || "dead".equalsIgnoreCase(status)) { + // The concurrent winner crashed before starting. Clean up so the name is free for + // the caller to retry the create; this process is now the new winner-in-waiting. + try { + dockerClient.removeContainerCmd(existing.getId()).withForce(true).exec(); + } catch (NotFoundException alreadyRemoved) { + // A concurrent loser (or Ryuk) already removed it. Fine, name is free either way. + } + return Optional.empty(); + } + + // Still "created" (or some other transient state): a concurrent winner is still + // mid create-and-start. Back off and re-inspect directly, rather than retrying the + // create call in the meantime. + if (Instant.now().isAfter(deadline)) { + throw new IllegalStateException( + "Timed out waiting for concurrently-created reusable container \"" + + name + + "\" (hash: " + + hash + + ") to finish starting" + ); + } + try { + Thread.sleep(backoffMillis); + } catch (InterruptedException interrupted) { + Thread.currentThread().interrupt(); + throw new IllegalStateException( + "Interrupted while waiting for reusable container to start", + interrupted + ); + } + backoffMillis = Math.min(backoffMillis * 2, 2000); + } + } + /** * Set any custom settings for the create command such as shared memory size. */ diff --git a/core/src/test/java/org/testcontainers/containers/ReusabilityNameConflictTest.java b/core/src/test/java/org/testcontainers/containers/ReusabilityNameConflictTest.java new file mode 100644 index 00000000000..c3509eb3e77 --- /dev/null +++ b/core/src/test/java/org/testcontainers/containers/ReusabilityNameConflictTest.java @@ -0,0 +1,256 @@ +package org.testcontainers.containers; + +import com.github.dockerjava.api.command.CreateContainerCmd; +import com.github.dockerjava.api.command.CreateContainerResponse; +import com.github.dockerjava.api.command.InspectContainerCmd; +import com.github.dockerjava.api.command.InspectContainerResponse; +import com.github.dockerjava.api.command.RemoveContainerCmd; +import com.github.dockerjava.api.exception.ConflictException; +import com.github.dockerjava.api.exception.NotFoundException; +import com.github.dockerjava.core.command.CreateContainerCmdImpl; +import org.junit.jupiter.api.Test; +import org.mockito.Answers; +import org.mockito.Mockito; +import org.testcontainers.TestImages; +import org.testcontainers.utility.TestcontainersConfiguration; + +import java.util.Collections; +import java.util.Map; +import java.util.concurrent.atomic.AtomicInteger; +import java.util.concurrent.atomic.AtomicReference; +import java.util.function.Supplier; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.assertThatThrownBy; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.ArgumentMatchers.startsWith; +import static org.mockito.Mockito.when; + +/** + * Covers the cross-process arbiter in {@code GenericContainer#createOrJoinReusableContainer}: + * when the JVM-local lock can't help (a different process/JVM raced us), the deterministic + * container name derived from the reuse hash makes Docker's own name-uniqueness constraint the + * tiebreaker. These tests simulate the name conflict that a concurrent process's "docker create" + * would produce, without needing an actual second process. + * + *

Since the real reuse hash depends on the fully-configured {@code CreateContainerCmd} (labels, + * host config, etc. are only set by {@code applyConfiguration} right before hashing), these tests + * capture the hash {@code GenericContainer} actually computes, via {@link GenericContainer#HASH_LABEL} + * on the command passed to the mocked "create" call, rather than recomputing it independently, + * which would silently drift from the real value and invalidate the label-matching assertions. + */ +class ReusabilityNameConflictTest extends ReusabilityUnitTests.AbstractReusabilityTest { + + @Test + void reusesConflictingContainerWhenLabelMatchesAndRunning() { + Mockito.doReturn(true).when(TestcontainersConfiguration.getInstance()).environmentSupportsReuse(); + + String winnerContainerId = "winner-container-id"; + AtomicReference capturedHash = new AtomicReference<>(); + AtomicInteger createAttempts = new AtomicInteger(); + + when(client.createContainerCmd(any())).then(conflictOnceThenCapture(createAttempts, capturedHash)); + when(client.listContainersCmd()).then(listContainersAnswer()); + when(client.inspectContainerCmd(startsWith(GenericContainer.REUSE_CONTAINER_NAME_PREFIX))) + .then(inspectAnswer(() -> inspectResponse(winnerContainerId, "running", labelsFor(capturedHash.get())))); + // Once reused, tryStart() inspects by container id (not by the reuse name) to wait for + // mapped ports, so this needs its own stub distinct from the name-based one above. + when(client.inspectContainerCmd(winnerContainerId)).then(inv -> inspectContainerAnswer().answer(inv)); + + GenericContainer container = makeReusable(new GenericContainer<>(TestImages.TINY_IMAGE)); + container.start(); + + assertThat(container.getContainerId()).isEqualTo(winnerContainerId); + assertThat(createAttempts.get()) + .as("create attempts before finding the running winner") + .isGreaterThanOrEqualTo(1); + } + + @Test + void refusesToReuseConflictingContainerWithMismatchedLabel() { + Mockito.doReturn(true).when(TestcontainersConfiguration.getInstance()).environmentSupportsReuse(); + + when(client.createContainerCmd(any())) + .then(invocation -> { + CreateContainerCmd.Exec exec = command -> { + throw new ConflictException("container name already in use"); + }; + return new CreateContainerCmdImpl(exec, null, "image:latest"); + }); + when(client.listContainersCmd()).then(listContainersAnswer()); + when(client.inspectContainerCmd(startsWith(GenericContainer.REUSE_CONTAINER_NAME_PREFIX))) + .then( + inspectAnswer(() -> { + return inspectResponse("unrelated-container-id", "running", labelsFor("some-other-hash-entirely")); + }) + ); + + GenericContainer container = makeReusable(new GenericContainer<>(TestImages.TINY_IMAGE)); + + // doStart() retries tryStart() via Unreliables.retryUntilSuccess, so the IllegalStateException + // thrown by createOrJoinReusableContainer ends up as the *root* cause, wrapped first in a + // ContainerLaunchException by tryStart()'s catch block and then again after the retry gives up. + assertThatThrownBy(container::start) + .hasRootCauseInstanceOf(IllegalStateException.class) + .hasStackTraceContaining("is already in use by a container that is not a Testcontainers reuse container"); + } + + @Test + void retriesWhileConflictingContainerIsStillBeingCreated() { + Mockito.doReturn(true).when(TestcontainersConfiguration.getInstance()).environmentSupportsReuse(); + + String winnerContainerId = "winner-container-id"; + AtomicReference capturedHash = new AtomicReference<>(); + AtomicInteger createAttempts = new AtomicInteger(); + AtomicInteger inspectCalls = new AtomicInteger(); + + when(client.createContainerCmd(any())).then(conflictOnceThenCapture(createAttempts, capturedHash)); + when(client.listContainersCmd()).then(listContainersAnswer()); + + // First two inspects see the winner still in "created" state (not started yet), the third + // sees it "running". The retry/backoff loop must ride this out rather than giving up or + // misreporting a name clash. + when(client.inspectContainerCmd(startsWith(GenericContainer.REUSE_CONTAINER_NAME_PREFIX))) + .then( + inspectAnswer(() -> { + int call = inspectCalls.incrementAndGet(); + String status = call < 3 ? "created" : "running"; + return inspectResponse(winnerContainerId, status, labelsFor(capturedHash.get())); + }) + ); + when(client.inspectContainerCmd(winnerContainerId)).then(inv -> inspectContainerAnswer().answer(inv)); + + GenericContainer container = makeReusable(new GenericContainer<>(TestImages.TINY_IMAGE)); + container.start(); + + assertThat(container.getContainerId()).isEqualTo(winnerContainerId); + assertThat(inspectCalls.get()).as("number of re-inspects while waiting").isGreaterThanOrEqualTo(3); + } + + @Test + void cleansUpDeadConflictingContainerThenRetriesCreate() { + Mockito.doReturn(true).when(TestcontainersConfiguration.getInstance()).environmentSupportsReuse(); + + String deadContainerId = "dead-container-id"; + String freshContainerId = "fresh-container-id"; + AtomicReference capturedHash = new AtomicReference<>(); + AtomicInteger createAttempts = new AtomicInteger(); + AtomicInteger removeAttempts = new AtomicInteger(); + + when(client.createContainerCmd(any())) + .then(invocation -> { + CreateContainerCmd.Exec exec = command -> { + if (capturedHash.get() == null) { + capturedHash.set(command.getLabels().get(GenericContainer.HASH_LABEL)); + } + if (createAttempts.incrementAndGet() == 1) { + throw new ConflictException("container name already in use"); + } + CreateContainerResponse response = new CreateContainerResponse(); + response.setId(freshContainerId); + return response; + }; + return new CreateContainerCmdImpl(exec, null, "image:latest"); + }); + when(client.listContainersCmd()).then(listContainersAnswer()); + when(client.startContainerCmd(freshContainerId)).then(inv -> startContainerAnswer().answer(inv)); + when(client.inspectContainerCmd(freshContainerId)).then(inv -> inspectContainerAnswer().answer(inv)); + when(client.inspectContainerCmd(startsWith(GenericContainer.REUSE_CONTAINER_NAME_PREFIX))) + .then(inspectAnswer(() -> inspectResponse(deadContainerId, "exited", labelsFor(capturedHash.get())))); + + RemoveContainerCmd removeCmd = Mockito.mock(RemoveContainerCmd.class, Answers.RETURNS_SELF); + when(removeCmd.exec()) + .then(invocation -> { + removeAttempts.incrementAndGet(); + return null; + }); + when(client.removeContainerCmd(deadContainerId)).thenReturn(removeCmd); + + GenericContainer container = makeReusable(new GenericContainer<>(TestImages.TINY_IMAGE)); + container.start(); + + assertThat(container.getContainerId()).isEqualTo(freshContainerId); + assertThat(removeAttempts.get()).as("dead conflicting container removed").isEqualTo(1); + assertThat(createAttempts.get()).as("create retried after cleanup").isEqualTo(2); + } + + @Test + void toleratesConflictingContainerAlreadyRemovedByAConcurrentCleanup() { + Mockito.doReturn(true).when(TestcontainersConfiguration.getInstance()).environmentSupportsReuse(); + + String freshContainerId = "fresh-container-id"; + AtomicInteger createAttempts = new AtomicInteger(); + + when(client.createContainerCmd(any())) + .then(invocation -> { + CreateContainerCmd.Exec exec = command -> { + if (createAttempts.incrementAndGet() == 1) { + throw new ConflictException("container name already in use"); + } + CreateContainerResponse response = new CreateContainerResponse(); + response.setId(freshContainerId); + return response; + }; + return new CreateContainerCmdImpl(exec, null, "image:latest"); + }); + when(client.listContainersCmd()).then(listContainersAnswer()); + when(client.startContainerCmd(freshContainerId)).then(inv -> startContainerAnswer().answer(inv)); + when(client.inspectContainerCmd(freshContainerId)).then(inv -> inspectContainerAnswer().answer(inv)); + + // Simulate a concurrent loser (or Ryuk) already removing the conflicting container by the + // time we inspect it: NotFoundException, not a running/created/exited container. The + // arbiter must treat this as "nothing to join, just retry create", not as an error. + when(client.inspectContainerCmd(startsWith(GenericContainer.REUSE_CONTAINER_NAME_PREFIX))) + .then(invocation -> { + InspectContainerCmd cmd = Mockito.mock(InspectContainerCmd.class); + when(cmd.exec()).thenThrow(new NotFoundException("no such container")); + return cmd; + }); + + GenericContainer container = makeReusable(new GenericContainer<>(TestImages.TINY_IMAGE)); + container.start(); + + assertThat(container.getContainerId()).isEqualTo(freshContainerId); + assertThat(createAttempts.get()).as("create retried after conflicting container vanished").isEqualTo(2); + } + + private static org.mockito.stubbing.Answer conflictOnceThenCapture( + AtomicInteger createAttempts, + AtomicReference capturedHash + ) { + return invocation -> { + CreateContainerCmd.Exec exec = command -> { + capturedHash.compareAndSet(null, command.getLabels().get(GenericContainer.HASH_LABEL)); + createAttempts.incrementAndGet(); + throw new ConflictException("container name already in use"); + }; + return new CreateContainerCmdImpl(exec, null, "image:latest"); + }; + } + + private static Map labelsFor(String hash) { + return Collections.singletonMap(GenericContainer.HASH_LABEL, hash); + } + + private static InspectContainerResponse inspectResponse(String id, String status, Map labels) { + InspectContainerResponse response = Mockito.mock(InspectContainerResponse.class, Answers.RETURNS_DEEP_STUBS); + when(response.getId()).thenReturn(id); + when(response.getConfig().getLabels()).thenReturn(labels); + when(response.getState().getStatus()).thenReturn(status); + return response; + } + + private static org.mockito.stubbing.Answer inspectAnswer( + Supplier resultSupplier + ) { + return invocation -> { + // Resolve the result before opening the when(...) stub: resultSupplier.get() creates + // and stubs its own mock (see inspectResponse()), and nesting that inside an + // in-progress when(cmd.exec()) call confuses Mockito's stubbing state. + InspectContainerResponse result = resultSupplier.get(); + InspectContainerCmd cmd = Mockito.mock(InspectContainerCmd.class); + when(cmd.exec()).thenReturn(result); + return cmd; + }; + } +} diff --git a/core/src/test/java/org/testcontainers/containers/ReusabilityRaceConditionTest.java b/core/src/test/java/org/testcontainers/containers/ReusabilityRaceConditionTest.java new file mode 100644 index 00000000000..cf237f844eb --- /dev/null +++ b/core/src/test/java/org/testcontainers/containers/ReusabilityRaceConditionTest.java @@ -0,0 +1,186 @@ +package org.testcontainers.containers; + +import com.fasterxml.jackson.core.type.TypeReference; +import com.fasterxml.jackson.databind.ObjectMapper; +import com.github.dockerjava.api.command.CreateContainerCmd; +import com.github.dockerjava.api.command.CreateContainerResponse; +import com.github.dockerjava.api.command.ListContainersCmd; +import com.github.dockerjava.api.model.Container; +import com.github.dockerjava.core.command.CreateContainerCmdImpl; +import com.github.dockerjava.core.command.ListContainersCmdImpl; +import org.junit.jupiter.api.Test; +import org.mockito.Mockito; +import org.testcontainers.TestImages; +import org.testcontainers.utility.TestcontainersConfiguration; + +import java.util.List; +import java.util.concurrent.CopyOnWriteArrayList; +import java.util.concurrent.CountDownLatch; +import java.util.concurrent.TimeUnit; +import java.util.stream.Collectors; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.Mockito.when; + +/** + * Covers the check-then-act race in {@link GenericContainer#findContainerForReuse(String)} that + * used to be marked with a "// TODO locking" comment: two containers with the identical reuse + * hash, started concurrently, could both miss the reuse lookup and both end up creating a new + * container, silently defeating the purpose of {@code withReuse(true)}. + * + *

{@link GenericContainer#tryStart()} now guards the "check reuse, else create" sequence with + * a per-hash JVM-local lock, which is what this test now verifies: exactly one real container is + * created for two same-JVM concurrent starts with an identical hash, and the second one reuses + * the first. The lock only protects same-JVM races; cross-process races are covered separately by + * {@code GenericContainer#createOrJoinReusableContainer}'s name-conflict handling, exercised in + * {@link ReusabilityNameConflictTest}. + */ +class ReusabilityRaceConditionTest extends ReusabilityUnitTests.AbstractReusabilityTest { + + // Simulates the Docker daemon's container list, updated only once a "create" call completes - + // exactly like the real docker daemon, there is no lock held between the "list" read and the "create" write. + private final List runningContainerIds = new CopyOnWriteArrayList<>(); + + @Test + void concurrentStartsWithIdenticalHashCreateExactlyOneContainer() throws InterruptedException { + Mockito.doReturn(true).when(TestcontainersConfiguration.getInstance()).environmentSupportsReuse(); + + when(client.listContainersCmd()) + .then(invocation -> { + ListContainersCmd.Exec exec = command -> { + return new ObjectMapper() + .convertValue( + runningContainerIds.stream().map(id -> singletonIdMap(id)).collect(Collectors.toList()), + new TypeReference>() {} + ); + }; + return new ListContainersCmdImpl(exec); + }); + + when(client.createContainerCmd(any())) + .then(invocation -> { + CreateContainerCmd.Exec exec = command -> { + // Simulate real-world latency of an actual "docker create" round trip, + // widening the window between the reuse lookup and the container becoming visible. + sleepQuietly(300); + String newId = "created-" + java.util.UUID.randomUUID(); + runningContainerIds.add(newId); + CreateContainerResponse response = new CreateContainerResponse(); + response.setId(newId); + return response; + }; + return new CreateContainerCmdImpl(exec, null, "image:latest"); + }); + + when(client.startContainerCmd(any())).then(inv -> startContainerAnswer().answer(inv)); + when(client.inspectContainerCmd(any())).then(inv -> inspectContainerAnswer().answer(inv)); + + GenericContainer containerA = makeReusable(new GenericContainer<>(TestImages.TINY_IMAGE)); + GenericContainer containerB = makeReusable(new GenericContainer<>(TestImages.TINY_IMAGE)); + + CountDownLatch bothReady = new CountDownLatch(2); + Runnable startAndCountDown = () -> { + bothReady.countDown(); + try { + bothReady.await(5, TimeUnit.SECONDS); + } catch (InterruptedException e) { + Thread.currentThread().interrupt(); + } + }; + + Thread t1 = new Thread(() -> { + startAndCountDown.run(); + containerA.start(); + }); + Thread t2 = new Thread(() -> { + startAndCountDown.run(); + containerB.start(); + }); + + t1.start(); + t2.start(); + t1.join(10_000); + t2.join(10_000); + + // Both containers were configured identically (same image, same reuse hash), and reuse + // was requested for both. With the per-hash JVM-local lock in tryStart(), the two threads + // are serialized around "check reuse, else create": whichever thread wins the lock first + // creates the real container, and the other thread's lookup (also inside the lock) then + // finds it and reuses it, so exactly one real container is ever created. + assertThat(runningContainerIds).as("number of containers actually created").hasSize(1); + assertThat(containerA.getContainerId()).isEqualTo(containerB.getContainerId()); + } + + @Test + void twoOrMoreThreadsRacingStillCreateExactlyOneContainer() throws InterruptedException { + Mockito.doReturn(true).when(TestcontainersConfiguration.getInstance()).environmentSupportsReuse(); + + when(client.listContainersCmd()) + .then(invocation -> { + ListContainersCmd.Exec exec = command -> { + return new ObjectMapper() + .convertValue( + runningContainerIds.stream().map(id -> singletonIdMap(id)).collect(Collectors.toList()), + new TypeReference>() {} + ); + }; + return new ListContainersCmdImpl(exec); + }); + + when(client.createContainerCmd(any())) + .then(invocation -> { + CreateContainerCmd.Exec exec = command -> { + sleepQuietly(100); + String newId = "created-" + java.util.UUID.randomUUID(); + runningContainerIds.add(newId); + CreateContainerResponse response = new CreateContainerResponse(); + response.setId(newId); + return response; + }; + return new CreateContainerCmdImpl(exec, null, "image:latest"); + }); + + when(client.startContainerCmd(any())).then(inv -> startContainerAnswer().answer(inv)); + when(client.inspectContainerCmd(any())).then(inv -> inspectContainerAnswer().answer(inv)); + + int threadCount = 5; + CountDownLatch allReady = new CountDownLatch(threadCount); + List> containers = new CopyOnWriteArrayList<>(); + List threads = new java.util.ArrayList<>(); + for (int i = 0; i < threadCount; i++) { + GenericContainer container = makeReusable(new GenericContainer<>(TestImages.TINY_IMAGE)); + containers.add(container); + Thread t = new Thread(() -> { + allReady.countDown(); + try { + allReady.await(5, TimeUnit.SECONDS); + } catch (InterruptedException e) { + Thread.currentThread().interrupt(); + } + container.start(); + }); + threads.add(t); + } + + threads.forEach(Thread::start); + for (Thread t : threads) { + t.join(10_000); + } + + assertThat(runningContainerIds).as("number of containers actually created").hasSize(1); + assertThat(containers.stream().map(GenericContainer::getContainerId).collect(Collectors.toSet())).hasSize(1); + } + + private static void sleepQuietly(long millis) { + try { + Thread.sleep(millis); + } catch (InterruptedException e) { + Thread.currentThread().interrupt(); + } + } + + private static java.util.Map singletonIdMap(String id) { + return java.util.Collections.singletonMap("Id", id); + } +}