This is an automated email from the ASF dual-hosted git repository. rmaucher pushed a commit to branch 9.0.x in repository https://gitbox.apache.org/repos/asf/tomcat.git
commit 7eb9c1acb81574ed1351bd85ac96db2b8cf5b378 Author: opencode <[email protected]> AuthorDate: Fri Oct 16 14:28:58 2026 +0200 Keep the acquisition count of LimitLatch stable across releaseAll() releaseAll() wakes the queued threads by releasing the synchronizer. The release path also decremented the acquisition counter by one, a decrement that corresponded to no thread returning a share, leaving the counter below the number of shares actually held. Callers that inspect the counter after releaseAll(), or that continue to use the latch rather than discarding it, saw fewer acquisitions counted than there were, so the limit could be exceeded once enforced again. releaseAll() now wakes the waiting threads without changing the counter. Threads released in this way hold a share until they call countDown(), like any other acquisition. The documentation of reset() now describes the consequence of resetting while shares are still held and that queued threads are not woken by a reset, and the documented return value of countDown() now matches the value it has always returned. Add testReleaseAllKeepsCount which pins the counter through a releaseAll() cycle with waiting threads. --- .../org/apache/tomcat/util/threads/LimitLatch.java | 21 ++++++--- .../apache/tomcat/util/threads/TestLimitLatch.java | 52 ++++++++++++++++++++++ 2 files changed, 66 insertions(+), 7 deletions(-) diff --git a/java/org/apache/tomcat/util/threads/LimitLatch.java b/java/org/apache/tomcat/util/threads/LimitLatch.java index 762c1d19eb..0cd5fc52bc 100644 --- a/java/org/apache/tomcat/util/threads/LimitLatch.java +++ b/java/org/apache/tomcat/util/threads/LimitLatch.java @@ -56,7 +56,10 @@ public class LimitLatch { @Override protected boolean tryReleaseShared(int arg) { - count.decrementAndGet(); + // An argument of zero is used for releaseAll() which must not change the acquisition count. + if (arg != 0) { + count.decrementAndGet(); + } return true; } } @@ -131,10 +134,10 @@ public class LimitLatch { /** * Releases a shared latch, making it available for another thread to use. * - * @return the previous counter value + * @return the counter value after the decrement */ public long countDown() { - sync.releaseShared(0); + sync.releaseShared(1); long result = getCount(); if (log.isTraceEnabled()) { log.trace("Counting down[" + Thread.currentThread().getName() + "] latch=" + result); @@ -143,9 +146,9 @@ public class LimitLatch { } /** - * Releases all waiting threads and causes the {@link #limit} to be ignored until {@link #reset()} is called. Note - * that this method also decrements the acquisition counter by one as a side effect, so {@link #reset()} should be - * called (or a new {@link LimitLatch} used) before the limit is enforced again. + * Releases all waiting threads and causes the {@link #limit} to be ignored until {@link #reset()} is called. This + * method does not change the acquisition counter. Threads that this method releases are counted as holding a share + * and each must still call {@link #countDown()} once it has finished, as with any other acquisition. * * @return <code>true</code> if release was done */ @@ -155,7 +158,11 @@ public class LimitLatch { } /** - * Resets the latch and initializes the shared acquisition counter to zero. + * Resets the latch and initializes the shared acquisition counter to zero. Any shares currently held are not + * accounted for. When the holders of those shares later call {@link #countDown()}, the counter goes negative which + * effectively allows that many additional acquisitions before the limit is reached again. Reset the latch only when + * no shares are held (for example, after using a dedicated latch for a period during which the limit is ignored), + * or accept that the limit is not reliably enforced until the counter has returned to zero. * * @see #releaseAll() */ diff --git a/test/org/apache/tomcat/util/threads/TestLimitLatch.java b/test/org/apache/tomcat/util/threads/TestLimitLatch.java index 4538f51aed..e7cba8c1fb 100644 --- a/test/org/apache/tomcat/util/threads/TestLimitLatch.java +++ b/test/org/apache/tomcat/util/threads/TestLimitLatch.java @@ -85,6 +85,58 @@ public class TestLimitLatch { checkWaitingThreadCount(latch, 0); } + @Test + public void testReleaseAllKeepsCount() throws Exception { + LimitLatch latch = new LimitLatch(2); + Object lock = new Object(); + + // Two threads hold a share of the latch + latch.countUpOrAwait(); + latch.countUpOrAwait(); + + // Five threads wait for a share of the latch + TestThread[] testThreads = new TestThread[5]; + for (int i = 0; i < testThreads.length; i++) { + testThreads[i] = new TestThread(latch, lock); + testThreads[i].start(); + } + for (int i = 0; i < testThreads.length; i++) { + if (!waitForThreadToStart(testThreads[i])) { + Assert.fail("Test thread [" + i + "] did not start"); + } + } + checkWaitingThreadCount(latch, 5); + + // Releasing all waiting threads must not change the count. The released threads acquire a share which they + // return below. + latch.releaseAll(); + + if (!waitForThreadsToReachStage(testThreads, 0, 5, 0)) { + Assert.fail("Threads were not released"); + } + // Two shares held by this thread plus one acquired by each of the released threads + Assert.assertEquals(7, latch.getCount()); + + synchronized (lock) { + lock.notifyAll(); + } + + if (!waitForThreadsToReachStage(testThreads, 0, 0, 5)) { + Assert.fail("Threads did not complete"); + } + Assert.assertEquals(2, latch.getCount()); + + // Return the shares held by this thread + latch.countDown(); + latch.countDown(); + Assert.assertEquals(0, latch.getCount()); + + latch.reset(); + latch.countUpOrAwait(); + Assert.assertEquals(1, latch.getCount()); + } + + @Test public void testTenWait() throws Exception { LimitLatch latch = new LimitLatch(10); --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
