This is an automated email from the ASF dual-hosted git repository. rmaucher pushed a commit to branch main in repository https://gitbox.apache.org/repos/asf/tomcat.git
commit a37307ea45a595010996c66f313ae64c4113ef6f Author: opencode <[email protected]> AuthorDate: Fri Oct 16 14:21:04 2026 +0200 Avoid busy spin in TaskQueue.take() when the keep-alive time is zero A worker whose thread is due to be stopped waits with a timed poll so that the check for whether the thread should stop runs periodically. The poll used the pool keep-alive time as the timeout. That time can be configured to zero, in which case the poll can not block and, for a core thread that is not subject to being culled, the wait degenerates into a loop that consumes a full CPU core while the queue is empty. When the configured timeout rounds down to zero, wait for the thread renewal delay instead. The interval at which a thread may stop itself is gated by that same delay, so the check still runs at a bounded interval without the spin, arriving tasks are still picked up immediately and the wait remains interruptible. Timed waits for non-core workers, which rely on a zero keep-alive time to be culled immediately, are not affected. Add TestTaskQueue with a regression test that counts the poll iterations performed by idle, renewal-due workers. Without the bounded timeout the count reaches millions within the test window. --- java/org/apache/tomcat/util/threads/TaskQueue.java | 11 ++- .../apache/tomcat/util/threads/TestTaskQueue.java | 92 ++++++++++++++++++++++ 2 files changed, 102 insertions(+), 1 deletion(-) diff --git a/java/org/apache/tomcat/util/threads/TaskQueue.java b/java/org/apache/tomcat/util/threads/TaskQueue.java index 3f69271aa0..d906d7a789 100644 --- a/java/org/apache/tomcat/util/threads/TaskQueue.java +++ b/java/org/apache/tomcat/util/threads/TaskQueue.java @@ -138,7 +138,16 @@ public class TaskQueue extends LinkedBlockingQueue<Runnable> implements Retryabl @Override public Runnable take() throws InterruptedException { if (parent != null && parent.currentThreadShouldBeStopped()) { - return poll(parent.getKeepAliveTime(TimeUnit.MILLISECONDS), TimeUnit.MILLISECONDS); + long timeout = parent.getKeepAliveTime(TimeUnit.MILLISECONDS); + if (timeout <= 0) { + /* + * A timed wait with a zero timeout can not block. Waiting for the thread renewal delay instead keeps + * the check for whether the thread should stop at a bounded interval without consuming CPU while the + * queue is empty. + */ + timeout = Math.max(1, parent.getThreadRenewalDelay()); + } + return poll(timeout, TimeUnit.MILLISECONDS); // yes, this may return null (in case of timeout) which normally // does not occur with take() // but the ThreadPoolExecutor implementation allows this diff --git a/test/org/apache/tomcat/util/threads/TestTaskQueue.java b/test/org/apache/tomcat/util/threads/TestTaskQueue.java new file mode 100644 index 0000000000..6d117b9cce --- /dev/null +++ b/test/org/apache/tomcat/util/threads/TestTaskQueue.java @@ -0,0 +1,92 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.apache.tomcat.util.threads; + +import java.util.concurrent.CountDownLatch; +import java.util.concurrent.TimeUnit; +import java.util.concurrent.atomic.AtomicInteger; + +import org.junit.Assert; +import org.junit.Test; + +public class TestTaskQueue { + + private static class CountingTaskQueue extends TaskQueue { + + private final AtomicInteger pollCount = new AtomicInteger(); + + @Override + public Runnable poll(long timeout, TimeUnit unit) throws InterruptedException { + pollCount.incrementAndGet(); + return super.poll(timeout, unit); + } + } + + + /* + * A thread that is due to be stopped waits with poll() rather than take() so the stop check runs periodically. If + * the keep-alive time is zero, a poll() with a zero timeout can not block and the worker spins while the queue is + * empty. The wait must be bounded to a non-zero interval instead. + */ + @Test + public void testTakeWithZeroKeepAliveDoesNotSpin() throws Exception { + CountingTaskQueue queue = new CountingTaskQueue(); + /* + * Two workers so that one stops itself (the first to claim the renewal slot) while the other remains due for + * renewal and waiting. A long renewal delay keeps the waiting thread renewing rather than stopping during the + * observation window below. + */ + ThreadPoolExecutor executor = new ThreadPoolExecutor(2, 2, 0, TimeUnit.MILLISECONDS, queue, + new TaskThreadFactory("test-take-zero", false, Thread.NORM_PRIORITY)); + queue.setParent(executor); + executor.setThreadRenewalDelay(10000); + + try { + CountDownLatch executed = new CountDownLatch(4); + executor.execute(executed::countDown); + executor.execute(executed::countDown); + // Wait for both workers to be created and to enter take() + Thread.sleep(200); + + // Mark the (older) worker threads as due to be stopped + executor.contextStopping(); + executor.execute(executed::countDown); + executor.execute(executed::countDown); + // The workers complete these tasks and then re-enter take() which, since the threads are due to be + // stopped, switches to poll() with a bounded timeout + Assert.assertTrue(executed.await(5, TimeUnit.SECONDS)); + + int pollsAfterTask = queue.pollCount.get(); + Thread.sleep(500); + int spinIterations = queue.pollCount.get() - pollsAfterTask; + + // Without the bounded timeout this is orders of magnitude larger (millions in this window) + Assert.assertTrue("Unexpected poll count while idle: " + spinIterations, spinIterations < 100); + + // Both workers must have entered the renewal wait at least once + Assert.assertTrue("Renewal wait was not exercised", queue.pollCount.get() >= 2); + + // The workers must still be able to process tasks + CountDownLatch processed = new CountDownLatch(1); + executor.execute(processed::countDown); + Assert.assertTrue(processed.await(5, TimeUnit.SECONDS)); + } finally { + executor.shutdownNow(); + Assert.assertTrue(executor.awaitTermination(10, TimeUnit.SECONDS)); + } + } +} --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
