This is an automated email from the ASF dual-hosted git repository.

rmaucher pushed a commit to branch 11.0.x
in repository https://gitbox.apache.org/repos/asf/tomcat.git

commit 726aa67c84451a1e4ed11b87d5c202b28abf8340
Author: opencode <[email protected]>
AuthorDate: Wed Oct 14 11:53:41 2026 +0200

    Enforce lowered limit immediately in SynchronizedStack.setLimit()
    
    setLimit() only stored the limit field while push() consulted the limit
    only when the backing array was already full. A stack with the default
    capacity of 128 and a newly configured limit of, say, 10 therefore kept
    accepting pushes until it held 128 elements, ignoring the limit. For
    the HTTP/2 recycled Request/Response pool, whose limit is set from the
    connector's maxConnections, this allowed the pool to hold up to 128
    entries when maxConnections was configured below 128.
    
    Fix this by shrinking the stack in setLimit() when the new limit is
    lower than the current capacity. The most recently pushed elements are
    retained and any remaining, older elements are discarded along with the
    old backing array, which releases their references.
    
    As follow-up work prompted by review, make expand() able to grow from a
    zero capacity array using Math.max(1, size * 2). Shrinking to a limit of
    zero and later raising the limit previously produced an
    ArrayIndexOutOfBoundsException on the next push, and the same latent
    problem existed for a stack constructed with a zero size. Add tests for
    shrinking below the capacity and below the element count, for the
    unlimited limit and for zero-then-raise. All assertions fail against the
    previous implementation where applicable.
---
 .../tomcat/util/collections/SynchronizedStack.java | 27 ++++++--
 .../util/collections/TestSynchronizedStack.java    | 75 ++++++++++++++++++++++
 2 files changed, 98 insertions(+), 4 deletions(-)

diff --git a/java/org/apache/tomcat/util/collections/SynchronizedStack.java 
b/java/org/apache/tomcat/util/collections/SynchronizedStack.java
index e6fccb9ec9..0f13f90e15 100644
--- a/java/org/apache/tomcat/util/collections/SynchronizedStack.java
+++ b/java/org/apache/tomcat/util/collections/SynchronizedStack.java
@@ -130,23 +130,42 @@ public class SynchronizedStack<T> {
 
     /**
      * Sets the maximum number of elements that can be pushed onto this stack.
+     * If the new limit is lower than the current capacity, the stack is
+     * shrunk. When the stack already holds more elements than the new limit,
+     * the most recently pushed elements are retained and the remaining
+     * elements are discarded.
      *
      * @param limit the maximum size, or -1 for unlimited
      */
     public synchronized void setLimit(int limit) {
         this.limit = limit;
+        if (limit > -1 && limit < size) {
+            // Keep the most recently pushed elements. They occupy
+            // stack[0..index] so the elements to keep are the last 'keep' of
+            // those.
+            int keep = Math.min(index + 1, limit);
+            Object[] newStack = new Object[limit];
+            System.arraycopy(stack, index + 1 - keep, newStack, 0, keep);
+            // The old array becomes garbage, releasing any references held by
+            // the discarded elements
+            stack = newStack;
+            size = limit;
+            index = keep - 1;
+        }
     }
 
     private void expand() {
-        int newSize = size * 2;
+        // At least one slot of growth is required to make progress. A size of
+        // zero can occur via the constructor or via setLimit(0).
+        int newSize = Math.max(1, size * 2);
         if (limit != -1 && newSize > limit) {
             newSize = limit;
         }
         Object[] newStack = new Object[newSize];
         System.arraycopy(stack, 0, newStack, 0, size);
-        // This is the only point where garbage is created by throwing away the
-        // old array. Note it is only the array, not the contents, that becomes
-        // garbage.
+        // This is one of the points where garbage is created by throwing away
+        // the old array (setLimit() being another). Note it is only the array,
+        // not the contents, that becomes garbage.
         stack = newStack;
         size = newSize;
     }
diff --git a/test/org/apache/tomcat/util/collections/TestSynchronizedStack.java 
b/test/org/apache/tomcat/util/collections/TestSynchronizedStack.java
index f112bcebd0..66dcc45043 100644
--- a/test/org/apache/tomcat/util/collections/TestSynchronizedStack.java
+++ b/test/org/apache/tomcat/util/collections/TestSynchronizedStack.java
@@ -116,4 +116,79 @@ public class TestSynchronizedStack {
 
         Assert.assertNull(stack.pop());
     }
+
+
+    @Test
+    public void testSetLimitBelowCapacity() {
+        SynchronizedStack<Object> stack = new SynchronizedStack<>();
+
+        Object o1 = new Object();
+        stack.push(o1);
+
+        // The new limit is below the current capacity but above the number of
+        // elements. All elements must be retained.
+        stack.setLimit(10);
+
+        Assert.assertSame(o1, stack.pop());
+        Assert.assertNull(stack.pop());
+
+        // The limit must now be enforced on pushes
+        for (int i = 0; i < 10; i++) {
+            Assert.assertTrue(stack.push(new Object()));
+        }
+        Assert.assertFalse(stack.push(new Object()));
+    }
+
+
+    @Test
+    public void testSetLimitBelowElementCount() {
+        SynchronizedStack<Object> stack = new SynchronizedStack<>();
+
+        int count = 50;
+        Object[] objects = new Object[count];
+        for (int i = 0; i < count; i++) {
+            objects[i] = new Object();
+            stack.push(objects[i]);
+        }
+
+        // The new limit is below the number of elements. The most recently
+        // pushed elements must be retained, the remainder discarded.
+        int limit = 10;
+        stack.setLimit(limit);
+
+        Assert.assertFalse(stack.push(new Object()));
+
+        for (int i = count - 1; i >= count - limit; i--) {
+            Assert.assertSame(objects[i], stack.pop());
+        }
+        Assert.assertNull(stack.pop());
+    }
+
+
+    @Test
+    public void testSetLimitUnlimited() {
+        SynchronizedStack<Object> stack = new SynchronizedStack<>();
+
+        stack.setLimit(-1);
+
+        for (int i = 0; i < 500; i++) {
+            Assert.assertTrue(stack.push(new Object()));
+        }
+    }
+
+
+    @Test
+    public void testSetLimitZeroThenIncrease() {
+        SynchronizedStack<Object> stack = new SynchronizedStack<>();
+
+        stack.setLimit(0);
+
+        Assert.assertFalse(stack.push(new Object()));
+
+        // A stack that has been shrunk to zero must be able to grow again
+        // when the limit is raised
+        stack.setLimit(10);
+
+        Assert.assertTrue(stack.push(new Object()));
+    }
 }


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to