Fix timing of thread pool shutdown to respect quarkus.thread-pool.shutdown-interrupt config property

[dgaffuri]

• Fixes: quarkus.thread-pool.shutdown-interrupt is not respected #49733

[gsmet]

I haven’t checked the logic but I find that the new variable names add to the confusion as I don’t find them very consistent.

I think we should name the ones coming from the config (after conversion to nanos): configuredShutdownMaxDelay and configuredInterruptMaxDelay (or something similar).
And have consistent naming for shutdown and interrupt when the two variables mean the same thing.

Does it make sense? I’m on my phone so it might not :)

[gsmet]

Oh and thanks a lot for having a look and creating a PR :)

[dgaffuri]

Changed to use more meaningful variable names. It seems to me that there's still a bug: if I don't specify shutdown-check-interval, like in

java -Dquarkus.thread-pool.shutdown-check-interval= -Dquarkus.thread-pool.shutdown-interrupt=20s '-Dquarkus.log.category."io.quarkus.thread-pool".level=DEBUG' -jar target/quarkus-app/quarkus-run.jar

shutdown-interrupt is ignored, and thread terminates after 60 seconds.

2025-09-08 14:06:59,926 INFO  [io.quarkus] (main) sample 1.0.0-SNAPSHOT on JVM (powered by Quarkus 999-SNAPSHOT) started in 0.303s. Listening on: http://0.0.0.0:8080
2025-09-08 14:06:59,929 INFO  [io.quarkus] (main) Profile prod activated.
2025-09-08 14:06:59,929 INFO  [io.quarkus] (main) Installed features: [cdi, rest, smallrye-context-propagation, vertx]
2025-09-08 14:06:59,943 DEBUG [io.qua.thread-pool] (main) loop: 1, shutdownTimeout: 60000000000, shutdownCheckInterval: 9223372036854775807, shutdownInterrupt: 20000000000
2025-09-08 14:07:59,947 INFO  [sam.GreetingResource] (executor-thread-1) interrupted
2025-09-08 14:07:59,947 WARN  [io.qua.thread-pool] (main) Thread pool shutdown failed: -1 threads still running
2025-09-08 14:07:59,953 INFO  [io.quarkus] (main) sample stopped in 60.022s

[quarkus-bot]

Status for workflow Quarkus CI

This is the status report for running Quarkus CI on commit 5d2d863.

✅ The latest workflow run for the pull request has completed successfully.

It should be safe to merge provided you have a look at the other checks in the summary.

You can consult the Develocity build scans.

---

Flaky tests - Develocity

⚙️ JVM Integration Tests - JDK 17

📦 integration-tests/oidc-code-flow

❌ io.quarkus.it.keycloak.CodeFlowTest.testTokenRefresh - History

• expected: <true> but was: <false> - org.opentest4j.AssertionFailedError

org.opentest4j.AssertionFailedError: expected: <true> but was: <false>
	at io.quarkus.it.keycloak.CodeFlowTest$3.call(CodeFlowTest.java:876)
	at io.quarkus.it.keycloak.CodeFlowTest$3.call(CodeFlowTest.java:863)
	at org.awaitility.core.CallableCondition$ConditionEvaluationWrapper.eval(CallableCondition.java:99)
	at org.awaitility.core.ConditionAwaiter$ConditionPoller.call(ConditionAwaiter.java:248)
	at org.awaitility.core.ConditionAwaiter$ConditionPoller.call(ConditionAwaiter.java:235)
	at java.base/java.util.concurrent.FutureTask.run(FutureTask.java:264)
	at java.base/java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1136)

• expected: <true> but was: <false> - org.opentest4j.AssertionFailedError

org.opentest4j.AssertionFailedError: expected: <true> but was: <false>
	at io.quarkus.it.keycloak.CodeFlowTest$3.call(CodeFlowTest.java:876)
	at io.quarkus.it.keycloak.CodeFlowTest$3.call(CodeFlowTest.java:863)
	at org.awaitility.core.CallableCondition$ConditionEvaluationWrapper.eval(CallableCondition.java:99)
	at org.awaitility.core.ConditionAwaiter$ConditionPoller.call(ConditionAwaiter.java:248)
	at org.awaitility.core.ConditionAwaiter$ConditionPoller.call(ConditionAwaiter.java:235)
	at java.base/java.util.concurrent.FutureTask.run(FutureTask.java:264)
	at java.base/java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1136)

[gsmet]

@dmlloyd from what I can see, you implemented this config so I marked you as reviewer.

[dmlloyd]

Unless I misunderstand something, I think the bug (which I introduced) is that we're not updating start so elapsed gets decreased by increasingly large amounts; if so, wouldn't this be a simpler fix:

diff --git core/runtime/src/main/java/io/quarkus/runtime/ExecutorRecorder.java core/runtime/src/main/java/io/quarkus/runtime/ExecutorRecorder.java
index cc275c5ceac..53bc49893a6 100644
--- core/runtime/src/main/java/io/quarkus/runtime/ExecutorRecorder.java
+++ core/runtime/src/main/java/io/quarkus/runtime/ExecutorRecorder.java
@@ -92,7 +92,7 @@ public void run() {
                             intervalRemaining, interruptRemaining);
                     try {
                         if (!executor.awaitTermination(Math.min(remaining, intervalRemaining), TimeUnit.NANOSECONDS)) {
-                            long elapsed = System.nanoTime() - start;
+                            long elapsed = -start + (start = System.nanoTime());
                             intervalRemaining -= elapsed;
                             remaining -= elapsed;
                             interruptRemaining -= elapsed;

[gsmet]

@dmlloyd I personally wouldn't go with such a cryptic syntax. Especially given we already missed a regression in there.

While more verbose, I think the new code is probably a bit clearer. But not my area so I will let you judge.

That being said, I think we need a fix merged.

[dmlloyd]

Sure, if it's too cryptic it can be simplified by introducing a temporary variable to represent the intermediate value:

long finished = System.nanoTime();
elapsed = finished - start;
start = finished;

But either way the fix should be no more than a few lines.

I suggest it because I think the proposed change is quite confusing due to the variable renames obscuring the meanings of the values.

[dmlloyd]

I opened #50423 due to lack of response on this PR.