You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@tomcat.apache.org by re...@apache.org on 2026/10/16 15:04:04 UTC
(tomcat) 20/24: Keep the acquisition count of LimitLatch stable across releaseAll()
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 f358a09a31be76a03bcda94938b6b99c7ed3e02a
Author: opencode <re...@apache.org>
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 dd08dbdb58..04c36ff41c 100644
--- a/java/org/apache/tomcat/util/threads/LimitLatch.java
+++ b/java/org/apache/tomcat/util/threads/LimitLatch.java
@@ -58,7 +58,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;
}
}
@@ -133,10 +136,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);
@@ -145,9 +148,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
*/
@@ -157,7 +160,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: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org