Skip to content

Fix duct-tape:1.0.8 Zombie thread#9241

Open
dadoonet wants to merge 4 commits intotestcontainers:mainfrom
dadoonet:zombie-threads
Open

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

Conversation

@dadoonet
Copy link
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
Copy link

austince commented Mar 3, 2026

@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.
dadoonet and 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
Contributor Author

dadoonet commented Mar 8, 2026

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
Member

rnorth commented Mar 8, 2026

@dadoonet I'll have a quick look now

@rnorth
Copy link
Member

rnorth commented Mar 8, 2026

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?

Sign up for free to 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.

3 participants