Skip to content

Commit d9a5d7f

Browse files
committed
Fix race condition in IsRunningStartupCheckStrategy with stale cached container state
IsRunningStartupCheckStrategy used container.getContainerInfo().getState() which returns stale cached state from the port-mapping check. If the container exits between the port-mapping check and the startup check, the stale 'running' state caused the startup check to pass prematurely, and the wait strategy would start on a crashed/removed container. Fix by using the cached state as a hint (fast path) but verifying it with a single live Docker inspect. If the live inspect confirms the container is running, return success immediately. If it shows a different state (stale cache), fall through to rate-limited polling. If the live inspect fails/timeout (e.g., Docker unresponsive on slow CI), gracefully fall back to trusting the cached state as the best available information. Also improve diagnostics: - Include containerId in 'Container is removed' error message - Handle NotFoundException gracefully when retrieving logs from a removed container during cleanup - Wrap stop() in try-catch to prevent cleanup failures from suppressing the original ContainerLaunchException Re-enable testCommandQuickExitFailure which was disabled due to this race, and add testQuickExitWithDifferentExitCode.
1 parent a4d3a03 commit d9a5d7f

4 files changed

Lines changed: 74 additions & 13 deletions

File tree

core/src/main/java/org/testcontainers/containers/GenericContainer.java

Lines changed: 19 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -495,7 +495,10 @@ private void tryStart() {
495495
}
496496

497497
if (inspectContainerResponse == null) {
498-
throw new IllegalStateException("Wait strategy failed. Container is removed", e);
498+
throw new IllegalStateException(
499+
"Wait strategy failed. Container " + containerId + " is removed",
500+
e
501+
);
499502
}
500503

501504
InspectContainerResponse.ContainerState state = inspectContainerResponse.getState();
@@ -538,14 +541,23 @@ private void tryStart() {
538541

539542
if (containerId != null) {
540543
// Log output if startup failed, either due to a container failure or exception (including timeout)
541-
final String containerLogs = getLogs();
544+
try {
545+
final String containerLogs = getLogs();
542546

543-
if (containerLogs.length() > 0) {
544-
logger().error("Log output from the failed container:\n{}", containerLogs);
545-
} else {
546-
logger().error("There are no stdout/stderr logs available for the failed container");
547+
if (containerLogs.length() > 0) {
548+
logger().error("Log output from the failed container:\n{}", containerLogs);
549+
} else {
550+
logger().error("There are no stdout/stderr logs available for the failed container");
551+
}
552+
} catch (NotFoundException e2) {
553+
logger().error("Could not retrieve logs for container {}: container not found", containerId);
554+
}
555+
556+
try {
557+
stop();
558+
} catch (Exception e2) {
559+
logger().debug("Failed to stop container {}", containerId, e2);
547560
}
548-
stop();
549561
}
550562

551563
throw new ContainerLaunchException("Could not create/start container", e);

core/src/main/java/org/testcontainers/containers/startupcheck/IsRunningStartupCheckStrategy.java

Lines changed: 26 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -11,13 +11,35 @@
1111
*/
1212
public class IsRunningStartupCheckStrategy extends StartupCheckStrategy {
1313

14-
@SuppressWarnings("deprecation")
1514
@Override
15+
@SuppressWarnings("deprecation")
1616
public boolean waitUntilStartupSuccessful(GenericContainer<?> container) {
17-
// Optimization: container already has the initial "after start" state, check it first
18-
if (checkState(container.getContainerInfo().getState()) == StartupStatus.SUCCESSFUL) {
19-
return true;
17+
InspectContainerResponse.ContainerState cachedState = container.getContainerInfo().getState();
18+
StartupStatus cachedStatus = checkState(cachedState);
19+
20+
if (cachedStatus == StartupStatus.SUCCESSFUL) {
21+
// Cached state shows the container as running/exited-success — verify with
22+
// one live Docker inspect to detect stale state (e.g., container crashed
23+
// between the port-mapping check and this startup check).
24+
try {
25+
if (
26+
checkStartupState(container.getDockerClient(), container.getContainerId()) ==
27+
StartupStatus.SUCCESSFUL
28+
) {
29+
return true;
30+
}
31+
// Live state doesn't match cached — container may have crashed.
32+
// Fall through to full rate-limited polling.
33+
} catch (Exception e) {
34+
// Live inspect failed (e.g., Docker timeout on slow CI) — trust
35+
// the cached state as the best available information.
36+
return true;
37+
}
38+
} else if (cachedStatus == StartupStatus.FAILED) {
39+
// Container already exited with a non-zero exit code
40+
return false;
2041
}
42+
2143
return super.waitUntilStartupSuccessful(container);
2244
}
2345

core/src/test/java/org/testcontainers/containers/startupcheck/IsRunningStartupCheckStrategyTest.java

Lines changed: 12 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,5 @@
11
package org.testcontainers.containers.startupcheck;
22

3-
import org.junit.jupiter.api.Disabled;
43
import org.junit.jupiter.api.Test;
54
import org.testcontainers.TestImages;
65
import org.testcontainers.containers.GenericContainer;
@@ -17,7 +16,6 @@ void testCommandQuickExitSuccess() {
1716
}
1817

1918
@Test
20-
@Disabled("This test can fail to throw an AssertionError if the container doesn't fail quickly enough")
2119
void testCommandQuickExitFailure() {
2220
try (GenericContainer container = new GenericContainer<>(TestImages.TINY_IMAGE).withCommand("/bin/false")) {
2321
assertThatThrownBy(container::start)
@@ -34,4 +32,16 @@ void testCommandStaysRunning() {
3432
container.start(); // should start with no Exception
3533
}
3634
}
35+
36+
@Test
37+
void testQuickExitWithDifferentExitCode() {
38+
try (
39+
GenericContainer container = new GenericContainer<>(TestImages.TINY_IMAGE)
40+
.withCommand("/bin/sh", "-c", "exit 42")
41+
) {
42+
assertThatThrownBy(container::start)
43+
.hasStackTraceContaining("Container startup failed")
44+
.hasStackTraceContaining("Container did not start correctly");
45+
}
46+
}
3747
}

modules/postgresql/src/test/java/org/testcontainers/postgresql/PostgreSQLContainerTest.java

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22

33
import org.junit.jupiter.api.Test;
44
import org.testcontainers.PostgreSQLTestImages;
5+
import org.testcontainers.containers.ContainerLaunchException;
56
import org.testcontainers.db.AbstractContainerDatabaseTest;
67

78
import java.sql.ResultSet;
@@ -11,6 +12,7 @@
1112

1213
import static org.assertj.core.api.Assertions.assertThat;
1314
import static org.assertj.core.api.Assertions.assertThatNoException;
15+
import static org.assertj.core.api.Assertions.assertThatThrownBy;
1416

1517
class PostgreSQLContainerTest extends AbstractContainerDatabaseTest {
1618
static {
@@ -122,6 +124,21 @@ void testWithAdditionalUrlParamInJdbcUrl() {
122124
}
123125
}
124126

127+
@Test
128+
void testContainerExitBeforeStartupCheck() {
129+
// Regression test for #11860: verify that a PostgreSQLContainer which exits before/during
130+
// startup produces a handled ContainerLaunchException from the startup check, not a
131+
// suppressed or cascading error from cleanup (getLogs/stop throwing NotFoundException).
132+
try (
133+
PostgreSQLContainer postgres = new PostgreSQLContainer(PostgreSQLTestImages.POSTGRES_TEST_IMAGE)
134+
.withCommand("/bin/false")
135+
) {
136+
assertThatThrownBy(postgres::start)
137+
.isInstanceOf(ContainerLaunchException.class)
138+
.hasStackTraceContaining("Container startup failed");
139+
}
140+
}
141+
125142
private void assertHasCorrectExposedAndLivenessCheckPorts(PostgreSQLContainer postgres) {
126143
assertThat(postgres.getExposedPorts()).containsExactly(PostgreSQLContainer.POSTGRESQL_PORT);
127144
assertThat(postgres.getLivenessCheckPortNumbers())

0 commit comments

Comments
 (0)