Fix duct-tape:1.0.8 Zombie thread#9241
Conversation
|
@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.
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>
|
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
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.htmlI can work on this but I think it's unrelated to the current PR. Should I? |
|
@dadoonet I'll have a quick look now |
|
OK, based on the japicmp report it's due to the removal of 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? |
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.ducttapeTC new package. And I removed the dependency on theduct-tapejar.Then I added a method
Timeouts.shutdown()which basically calls theshutdown()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!