Skip to content

Commit 5195ec7

Browse files
rnorthclaude
andcommitted
Make Timeouts executor lazily initialized and re-creatable
Replace the static final ExecutorService with a lazily-created one so that shutdown() doesn't permanently break subsequent container operations. The executor is re-created transparently on next use. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
1 parent e157f5a commit 5195ec7

2 files changed

Lines changed: 35 additions & 37 deletions

File tree

core/src/main/java/org/testcontainers/utility/ducttape/Timeouts.java

Lines changed: 20 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -9,20 +9,28 @@
99
*/
1010
public class Timeouts {
1111

12-
private static final ExecutorService EXECUTOR_SERVICE = Executors.newCachedThreadPool(new ThreadFactory() {
12+
private static final AtomicInteger THREAD_COUNTER = new AtomicInteger(0);
1313

14-
final AtomicInteger threadCounter = new AtomicInteger(0);
14+
private static final ThreadFactory THREAD_FACTORY = r -> {
15+
Thread thread = new Thread(r, "ducttape-" + THREAD_COUNTER.getAndIncrement());
16+
thread.setDaemon(true);
17+
return thread;
18+
};
1519

16-
@Override
17-
public Thread newThread(Runnable r) {
18-
Thread thread = new Thread(r, "ducttape-" + threadCounter.getAndIncrement());
19-
thread.setDaemon(true);
20-
return thread;
20+
private static volatile ExecutorService executorService;
21+
22+
private static synchronized ExecutorService getExecutorService() {
23+
if (executorService == null || executorService.isShutdown()) {
24+
executorService = Executors.newCachedThreadPool(THREAD_FACTORY);
2125
}
22-
});
26+
return executorService;
27+
}
2328

24-
public static void shutdown() {
25-
EXECUTOR_SERVICE.shutdown();
29+
public static synchronized void shutdown() {
30+
if (executorService != null) {
31+
executorService.shutdown();
32+
executorService = null;
33+
}
2634
}
2735

2836
/**
@@ -40,7 +48,7 @@ public static <T> T getWithTimeout(final int timeout, final TimeUnit timeUnit, f
4048

4149
check("timeout must be greater than zero", timeout > 0);
4250

43-
Future<T> future = EXECUTOR_SERVICE.submit(lambda);
51+
Future<T> future = getExecutorService().submit(lambda);
4452
return callFuture(timeout, timeUnit, future);
4553
}
4654

@@ -57,7 +65,7 @@ public static void doWithTimeout(final int timeout, final TimeUnit timeUnit, fin
5765

5866
check("timeout must be greater than zero", timeout > 0);
5967

60-
Future<?> future = EXECUTOR_SERVICE.submit(lambda);
68+
Future<?> future = getExecutorService().submit(lambda);
6169
callFuture(timeout, timeUnit, future);
6270
}
6371

core/src/test/java/org/testcontainers/utility/TimeoutsShutdownTest.java

Lines changed: 15 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -3,43 +3,33 @@
33
import org.junit.Test;
44
import org.testcontainers.utility.ducttape.Timeouts;
55

6-
import java.util.concurrent.RejectedExecutionException;
76
import java.util.concurrent.TimeUnit;
87

98
import static org.assertj.core.api.Assertions.assertThat;
10-
import static org.assertj.core.api.Assertions.assertThatThrownBy;
119

1210
/**
13-
* POC demonstrating the bug in this PR's approach.
14-
*
15-
* <p>The PR adds {@code Timeouts.shutdown()} (which calls {@code EXECUTOR_SERVICE.shutdown()})
16-
* and invokes it from {@code GenericContainer.stop()}. Since {@code EXECUTOR_SERVICE} is a
17-
* static final field shared across all callers, shutting it down after stopping one container
18-
* permanently breaks all subsequent Timeouts usage — including wait strategies, startup checks,
19-
* and {@code Unreliables.retryUntilSuccess} for any container started afterwards.</p>
20-
*
21-
* <p><b>Note:</b> This test intentionally shuts down the shared Timeouts executor, which is
22-
* an irreversible operation within a single JVM. It should be run in its own Gradle test worker
23-
* (e.g. via {@code --tests} filter) to avoid affecting other tests.</p>
11+
* Verifies that {@link Timeouts} works correctly across shutdown/reuse cycles.
12+
* After {@code shutdown()} the executor is re-created on next use.
2413
*/
25-
public class TimeoutsShutdownBugTest {
14+
public class TimeoutsShutdownTest {
2615

27-
/**
28-
* Calls {@code Timeouts.shutdown()} (as {@code GenericContainer.stop()} does), then
29-
* proves that subsequent Timeouts calls fail with {@link RejectedExecutionException}.
30-
*/
3116
@Test
32-
public void shutdownBreaksSubsequentTimeoutCalls() {
33-
// First call works fine — represents a container's wait strategy completing successfully
17+
public void timeoutsWorkAfterShutdown() {
18+
// First use
3419
String result1 = Timeouts.getWithTimeout(5, TimeUnit.SECONDS, () -> "container-1-ready");
3520
assertThat(result1).isEqualTo("container-1-ready");
3621

37-
// This is what GenericContainer.stop() does after stopping a container
22+
// Shutdown (as GenericContainer.stop() does)
3823
Timeouts.shutdown();
3924

40-
// Second call — represents starting another container after the first was stopped.
41-
// This fails because the static shared executor rejects new tasks after shutdown.
42-
assertThatThrownBy(() -> Timeouts.getWithTimeout(5, TimeUnit.SECONDS, () -> "container-2-ready"))
43-
.isInstanceOf(RejectedExecutionException.class);
25+
// Second use — should transparently create a fresh executor
26+
String result2 = Timeouts.getWithTimeout(5, TimeUnit.SECONDS, () -> "container-2-ready");
27+
assertThat(result2).isEqualTo("container-2-ready");
28+
29+
// Shutdown and use again to confirm repeatable
30+
Timeouts.shutdown();
31+
32+
String result3 = Timeouts.getWithTimeout(5, TimeUnit.SECONDS, () -> "container-3-ready");
33+
assertThat(result3).isEqualTo("container-3-ready");
4434
}
4535
}

0 commit comments

Comments
 (0)