Skip to content

Fix duct-tape:1.0.8 Zombie thread - #9241

Closed
dadoonet wants to merge 4 commits into
testcontainers:mainfrom
dadoonet:zombie-threads
Closed

Fix duct-tape:1.0.8 Zombie thread#9241
dadoonet wants to merge 4 commits into
testcontainers:mainfrom
dadoonet:zombie-threads

Conversation

@dadoonet

Copy link
Copy Markdown
Contributor

This is a naive implementation for the bug described in #9227.

I basically moved the code from https://github.com/rnorth/duct-tape/ to org.testcontainers.utility.ducttape TC new package. And I removed the dependency on the duct-tape jar.
Then I added a method Timeouts.shutdown() which basically calls the shutdown() method on the Thread.
Finally, when we stop a container, I'm calling this Timeouts.shutdown().

This should close the remaining thread.

Let me know what you think and if I should continue on this path or not.

Thanks!

@austince

austince commented Mar 3, 2026

Copy link
Copy Markdown

@dadoonet@eddumelendez , sorry for the ping -- is there any help that we could give to get this one merged?

I offer a genuine-human-spotted-and-crafted-bug-fix as initial offering testcontainers/testcontainers-site#252

1st step: move code from the archived repository
Related to testcontainers#9227.
dadoonetand others added 3 commits March 8, 2026 11:30
2nd step: call shutdown() when stopping the container
Related to testcontainers#9227.
Demonstrates that calling Timeouts.shutdown() (as GenericContainer.stop()
does in this PR) permanently kills the static shared ExecutorService,
causing all subsequent Timeouts usage to fail with
RejectedExecutionException. This breaks any test suite that stops one
container and then starts another.
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
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>
@dadoonet

Copy link
Copy Markdown
ContributorAuthor

I rebased all the work done by @rnorth and myself on main so I have a better view of what was added.

But, when running ./gradlew :testcontainers:check, it's failing with:

❯ ./gradlew :testcontainers:check
Configuration on demand is an incubating feature.
> Task :testcontainers:japicmp FAILED
> Task :testcontainers:delombok
warning: unknown enum constant org.immutables.value.Value.Style.ImplementationVisibility.PACKAGE
reason: class file for org.immutables.value.Value$Style$ImplementationVisibility not found
/Users/david/IdeaProjects/3rdparty/testcontainers-java/core/src/main/java/org/testcontainers/images/AgeBasedPullPolicy.java:14: warning: Generating equals/hashCode implementation but without a call to superclass, even though this class does not extend java.lang.Object. If this is intentional, add '@EqualsAndHashCode(callSuper=false)' to your type.
@Value
^
warning: unknown enum constant org.immutables.value.Value.Style.ImplementationVisibility.PACKAGE
reason: class file for org.immutables.value.Value$Style$ImplementationVisibility not found
warning: unknown enum constant org.immutables.value.Value.Style.ImplementationVisibility.PACKAGE
reason: class file for org.immutables.value.Value$Style$ImplementationVisibility not found
warning: unknown enum constant org.immutables.value.Value.Style.ImplementationVisibility.PACKAGE
reason: class file for org.immutables.value.Value$Style$ImplementationVisibility not found
warning: unknown enum constant org.immutables.value.Value.Style.ImplementationVisibility.PACKAGE
reason: class file for org.immutables.value.Value$Style$ImplementationVisibility not found
> Task :testcontainers:checkstyleMain FAILED
[ant:checkstyle] [ERROR] /Users/david/IdeaProjects/3rdparty/testcontainers-java/core/src/main/java/org/testcontainers/containers/wait/strategy/HttpWaitStrategy.java:37:62: L'utilisation des imports statiques est prohibé - org.testcontainers.utility.ducttape.Unreliables.retryUntilSuccess [AvoidStaticImport][ant:checkstyle] [ERROR] /Users/david/IdeaProjects/3rdparty/testcontainers-java/core/src/main/java/org/testcontainers/utility/ducttape/Unreliables.java:10:64: L'utilisation des imports statiques est prohibé - org.testcontainers.utility.ducttape.Preconditions.check [AvoidStaticImport]
[ant:checkstyle] [ERROR] /Users/david/IdeaProjects/3rdparty/testcontainers-java/core/src/main/java/org/testcontainers/utility/ducttape/RateLimiterBuilder.java:5:64: L'utilisation des imports statiques est prohibé - org.testcontainers.utility.ducttape.Preconditions.check [AvoidStaticImport][ant:checkstyle] [ERROR] /Users/david/IdeaProjects/3rdparty/testcontainers-java/core/src/main/java/org/testcontainers/utility/ducttape/RateLimiterBuilder.java:14:5: 'VARIABLE_DEF' doit être séparé de la ligne précédente. [EmptyLineSeparator][ant:checkstyle] [ERROR] /Users/david/IdeaProjects/3rdparty/testcontainers-java/core/src/main/java/org/testcontainers/utility/ducttape/RateLimiterBuilder.java:15:5: 'VARIABLE_DEF' doit être séparé de la ligne précédente. [EmptyLineSeparator][ant:checkstyle] [ERROR] /Users/david/IdeaProjects/3rdparty/testcontainers-java/core/src/main/java/org/testcontainers/utility/ducttape/Timeouts.java:3:28: L'utilisation des import.* est prohibé - java.util.concurrent.*. [AvoidStarImport]
[Incubating] Problems report is available at: file:///Users/david/IdeaProjects/3rdparty/testcontainers-java/build/reports/problems/problems-report.html
FAILURE: Build completed with 2 failures.
1: Task failed with an exception.
-----------
* What went wrong:
Execution failed for task ':testcontainers:japicmp'.
> A failure occurred while executing me.champeau.gradle.japicmp.JApiCmpWorkAction
> Detected binary changes.
- current: testcontainers.jar
- baseline: testcontainers-2.0.2.jar.
See failure report at file:///Users/david/IdeaProjects/3rdparty/testcontainers-java/core/build/reports/japi.html

I can work on this but I think it's unrelated to the current PR. Should I?

@rnorth

Copy link
Copy Markdown
Member

@dadoonet I'll have a quick look now

@rnorth

Copy link
Copy Markdown
Member

OK, based on the japicmp report it's due to the removal of
org.testcontainers.containers.wait.strategy.AbstractWaitStrategy's withRateLimiter(org.rnorth.ducttape.ratelimits.RateLimiter) public method and change of the return type of getRateLimiter() (which is a protected method).

It's correctly flagging that this is a (probably very low impact) breaking change. I'm guessing we want to avoid a major version increment just for this, but perhaps it might be tolerable in a minor release?
@eddumelendez@kiview what do you think?

@eddumelendez

Copy link
Copy Markdown
Member

Hi, sorry for the late reply. But, my main issue with merging the PR is maintaining another domain under Testcontainers source code. That's why in the past I suggested to move to another library.

austince added a commit to austince/testcontainers-java that referenced this pull request May 19, 2026
Removes the dependency on org.rnorth.duct-tape:duct-tape:1.0.8 by
re-implementing the small subset of duct-tape used by Testcontainers
inside org.testcontainers.utility. No new runtime dependencies are
introduced.
Internal package additions (org.testcontainers.utility):
- RateLimiter (abstract class) + RateLimiterBuilder: sleep-based pacing
with constant-throughput strategy. Method names doWhenReady/getWhenReady
match the historical duct-tape API so existing TC source-compat is
preserved.
- Timeouts.getWithTimeout / doWithTimeout: per-call ExecutorService with
daemon worker thread, shutdownNow() in finally — fixes the duct-tape
zombie-thread bug (testcontainers#9227).
- Unreliables.retryUntilSuccess / retryUntilTrue: retry loop wrapped in
Timeouts.getWithTimeout. Interrupt-induced exceptions are not allowed
to overwrite the genuine last failure so callers diagnose the real
cause (e.g. SSLHandshakeException) instead of the cancellation.
- TimeoutException, RetryCountExceededException: unchecked, matching
duct-tape's semantics so existing callers compile unchanged. Message
"Retry limit hit with exception" is preserved verbatim for
GenericContainerTest's existing assertion.
Public API impact:
- AbstractWaitStrategy.getRateLimiter() and withRateLimiter() now take
org.testcontainers.utility.RateLimiter instead of
org.rnorth.ducttape.ratelimits.RateLimiter. Source-compat is preserved
for callers that go through AbstractWaitStrategy; only callers that
explicitly imported the duct-tape types break.
- Both methods are added to japicmp's methodExcludes with a comment
explaining the binary-breaking change.
Callers updated to use the new types: GenericContainer,
HttpWaitStrategy, ShellStrategy, DockerHealthcheckWaitStrategy,
WaitAllStrategy, StartupCheckStrategy, DockerClientProviderStrategy,
RyukResourceReaper, CassandraQueryWaitStrategy (both packages),
YugabyteDBY{CQL,SQL}WaitStrategy, and the corresponding tests.
Refs: testcontainers#9241
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
austince added a commit to austince/testcontainers-java that referenced this pull request May 19, 2026
Removes the dependency on org.rnorth.duct-tape:duct-tape:1.0.8 by
re-implementing the small subset of duct-tape used by Testcontainers
inside org.testcontainers.utility, backed by resilience4j modules per
maintainer preference in testcontainers#9241.
New runtime dependencies (shaded):
- io.github.resilience4j:resilience4j-timelimiter:1.7.1
- io.github.resilience4j:resilience4j-ratelimiter:1.7.1
Internal package additions (org.testcontainers.utility):
- RateLimiter (interface) + RateLimiterBuilder: thin facade with the
historical duct-tape method names doWhenReady / getWhenReady, backed
by a resilience4j RateLimiter adapter. The shaded resilience4j type
never leaks into the public API.
- Timeouts.getWithTimeout / doWithTimeout: per-call ExecutorService
with daemon worker thread, shutdownNow() in finally and resilience4j
TimeLimiter (cancelRunningFuture=true) — fixes the duct-tape
zombie-thread bug (testcontainers#9227).
- Unreliables.retryUntilSuccess / retryUntilTrue: retry loop wrapped
in Timeouts.getWithTimeout. Interrupt-induced exceptions
(resilience4j's AcquirePermissionCancelledException, etc.) are not
allowed to overwrite the genuine last failure so callers diagnose
the real cause (e.g. SSLHandshakeException).
- TimeoutException, RetryCountExceededException: unchecked, matching
duct-tape's semantics so existing callers compile unchanged. Message
"Retry limit hit with exception" is preserved verbatim for
GenericContainerTest's existing assertion.
Public API impact:
- AbstractWaitStrategy.getRateLimiter() and withRateLimiter() now take
org.testcontainers.utility.RateLimiter instead of
org.rnorth.ducttape.ratelimits.RateLimiter. Source-compat is preserved
for callers that go through AbstractWaitStrategy; only callers that
explicitly imported the duct-tape types break.
- Both methods are added to japicmp's methodExcludes with a comment
explaining the binary-breaking change.
Callers updated to use the new types: GenericContainer,
HttpWaitStrategy, ShellStrategy, DockerHealthcheckWaitStrategy,
WaitAllStrategy, StartupCheckStrategy, DockerClientProviderStrategy,
RyukResourceReaper, CassandraQueryWaitStrategy (both packages),
YugabyteDBY{CQL,SQL}WaitStrategy, and the corresponding tests.
Refs: testcontainers#9241
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
austince added a commit to austince/testcontainers-java that referenced this pull request May 19, 2026
Removes the dependency on org.rnorth.duct-tape:duct-tape:1.0.8 by
re-implementing the small subset of duct-tape used by Testcontainers
inside org.testcontainers.utility, backed by resilience4j modules per
maintainer preference in testcontainers#9241.
New runtime dependencies (shaded):
- io.github.resilience4j:resilience4j-timelimiter:1.7.1
- io.github.resilience4j:resilience4j-ratelimiter:1.7.1
Internal package additions (org.testcontainers.utility):
- RateLimiter (interface) + RateLimiterBuilder: thin facade with the
historical duct-tape method names doWhenReady / getWhenReady, backed
by a resilience4j RateLimiter adapter. The shaded resilience4j type
never leaks into the public API.
- Timeouts.getWithTimeout / doWithTimeout: per-call ExecutorService
with daemon worker thread, shutdownNow() in finally and resilience4j
TimeLimiter (cancelRunningFuture=true) — fixes the duct-tape
zombie-thread bug (testcontainers#9227).
- Unreliables.retryUntilSuccess / retryUntilTrue: retry loop wrapped
in Timeouts.getWithTimeout. Interrupt-induced exceptions
(resilience4j's AcquirePermissionCancelledException, etc.) are not
allowed to overwrite the genuine last failure so callers diagnose
the real cause (e.g. SSLHandshakeException).
- TimeoutException, RetryCountExceededException: unchecked, matching
duct-tape's semantics so existing callers compile unchanged. Message
"Retry limit hit with exception" is preserved verbatim for
GenericContainerTest's existing assertion.
Public API impact:
- AbstractWaitStrategy.getRateLimiter() and withRateLimiter() now take
org.testcontainers.utility.RateLimiter instead of
org.rnorth.ducttape.ratelimits.RateLimiter. Source-compat is preserved
for callers that go through AbstractWaitStrategy; only callers that
explicitly imported the duct-tape types break.
- Both methods are added to japicmp's methodExcludes with a comment
explaining the binary-breaking change.
Callers updated to use the new types: GenericContainer,
HttpWaitStrategy, ShellStrategy, DockerHealthcheckWaitStrategy,
WaitAllStrategy, StartupCheckStrategy, DockerClientProviderStrategy,
RyukResourceReaper, CassandraQueryWaitStrategy (both packages),
YugabyteDBY{CQL,SQL}WaitStrategy, and the corresponding tests.
Refs: testcontainers#9241
austince added a commit to austince/testcontainers-java that referenced this pull request May 19, 2026
Removes the dependency on org.rnorth.duct-tape:duct-tape:1.0.8 by
re-implementing the small subset of duct-tape used by Testcontainers
inside org.testcontainers.utility. No new runtime dependencies are
introduced.
Internal package additions (org.testcontainers.utility):
- RateLimiter (abstract class) + RateLimiterBuilder: sleep-based pacing
with constant-throughput strategy. Method names doWhenReady/getWhenReady
match the historical duct-tape API so existing TC source-compat is
preserved.
- Timeouts.getWithTimeout / doWithTimeout: per-call ExecutorService with
daemon worker thread, shutdownNow() in finally — fixes the duct-tape
zombie-thread bug (testcontainers#9227).
- Unreliables.retryUntilSuccess / retryUntilTrue: retry loop wrapped in
Timeouts.getWithTimeout. Interrupt-induced exceptions are not allowed
to overwrite the genuine last failure so callers diagnose the real
cause (e.g. SSLHandshakeException) instead of the cancellation.
- TimeoutException, RetryCountExceededException: unchecked, matching
duct-tape's semantics so existing callers compile unchanged. Message
"Retry limit hit with exception" is preserved verbatim for
GenericContainerTest's existing assertion.
Public API impact:
- AbstractWaitStrategy.getRateLimiter() and withRateLimiter() now take
org.testcontainers.utility.RateLimiter instead of
org.rnorth.ducttape.ratelimits.RateLimiter. Source-compat is preserved
for callers that go through AbstractWaitStrategy; only callers that
explicitly imported the duct-tape types break.
- Both methods are added to japicmp's methodExcludes with a comment
explaining the binary-breaking change.
Callers updated to use the new types: GenericContainer,
HttpWaitStrategy, ShellStrategy, DockerHealthcheckWaitStrategy,
WaitAllStrategy, StartupCheckStrategy, DockerClientProviderStrategy,
RyukResourceReaper, CassandraQueryWaitStrategy (both packages),
YugabyteDBY{CQL,SQL}WaitStrategy, and the corresponding tests.
Refs: testcontainers#9241
austince added a commit to austince/testcontainers-java that referenced this pull request May 19, 2026
Removes the dependency on org.rnorth.duct-tape:duct-tape:1.0.8 by
re-implementing the small subset of duct-tape used by Testcontainers
inside org.testcontainers.utility. No new runtime dependencies are
introduced.
Internal package additions (org.testcontainers.utility):
- RateLimiter (abstract class) + RateLimiterBuilder: sleep-based pacing
with constant-throughput strategy. Method names doWhenReady/getWhenReady
match the historical duct-tape API so existing TC source-compat is
preserved.
- Timeouts.getWithTimeout / doWithTimeout: per-call ExecutorService with
daemon worker thread, shutdownNow() in finally — fixes the duct-tape
zombie-thread bug (testcontainers#9227).
- Unreliables.retryUntilSuccess / retryUntilTrue: retry loop wrapped in
Timeouts.getWithTimeout. Interrupt-induced exceptions are not allowed
to overwrite the genuine last failure so callers diagnose the real
cause (e.g. SSLHandshakeException) instead of the cancellation.
- TimeoutException, RetryCountExceededException: unchecked, matching
duct-tape's semantics so existing callers compile unchanged. Message
"Retry limit hit with exception" is preserved verbatim for
GenericContainerTest's existing assertion.
Public API impact:
- AbstractWaitStrategy.getRateLimiter() and withRateLimiter() now take
org.testcontainers.utility.RateLimiter instead of
org.rnorth.ducttape.ratelimits.RateLimiter. Source-compat is preserved
for callers that go through AbstractWaitStrategy; only callers that
explicitly imported the duct-tape types break.
- Both methods are added to japicmp's methodExcludes with a comment
explaining the binary-breaking change.
Callers updated to use the new types: GenericContainer,
HttpWaitStrategy, ShellStrategy, DockerHealthcheckWaitStrategy,
WaitAllStrategy, StartupCheckStrategy, DockerClientProviderStrategy,
RyukResourceReaper, CassandraQueryWaitStrategy (both packages),
YugabyteDBY{CQL,SQL}WaitStrategy, and the corresponding tests.
Refs: testcontainers#9241
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants

@dadoonet@austince@rnorth@eddumelendez