This is an automated email from the ASF dual-hosted git repository. rmaucher pushed a commit to branch 10.1.x in repository https://gitbox.apache.org/repos/asf/tomcat.git
commit a0be92238bba265f577264d8f43f55924ff20dd6 Author: opencode <[email protected]> AuthorDate: Mon Nov 2 16:07:34 2026 +0100 Bring the fair queue's documentation up to date with its design The class javadoc still described the wait() era: waiting is done on a per-waiter CountDownLatch registered under the shared ReentrantLock, not in a "suspended monitor state", and the fairness guarantee is scoped to the order of waiter registration - a not-yet-registered caller may still barge ahead racing for the shared lock. The "not all methods are implemented" sentence was wrong the other way: all are implemented, a dozen deliberately throw UnsupportedOperationException, which is now the wording. The offer(e,timeout,unit) javadoc stopped mid-sentence; it now states that the timeout is never applied and the declared InterruptedException is never thrown. The isLinux rationale credited the poll(long,TimeUnit) operation with the reordering, but it is offer(E) that issues the reordered countDown() when it completes a poll's wait; the lock field's javadoc still promised per-thread locks, which the latch design no longer hands out. take() had no javadoc at all and now points at the delegation whose contract it inherits, including the cancelled-queue wait end that cancelWaiters() documents. The two section boundary comments had drifted: isEmpty() sits with the methods the pool uses and offer(e,timeout,unit) with the interface conformance group, so both statements are true again as written. Comments that only restated the line below them were removed; the rationale comments were kept. The cancelWaiters() javadoc, whose over-claim about timed waiters this batch was to fix, had already been corrected together with the behavior it describes by the pool close-race fix. --- .../apache/tomcat/jdbc/pool/FairBlockingQueue.java | 66 ++++++++++------------ 1 file changed, 31 insertions(+), 35 deletions(-) diff --git a/modules/jdbc-pool/src/main/java/org/apache/tomcat/jdbc/pool/FairBlockingQueue.java b/modules/jdbc-pool/src/main/java/org/apache/tomcat/jdbc/pool/FairBlockingQueue.java index 7d2bc5815d..2f08f4efaa 100644 --- a/modules/jdbc-pool/src/main/java/org/apache/tomcat/jdbc/pool/FairBlockingQueue.java +++ b/modules/jdbc-pool/src/main/java/org/apache/tomcat/jdbc/pool/FairBlockingQueue.java @@ -35,11 +35,13 @@ import java.util.concurrent.locks.ReentrantLock; import org.apache.tomcat.jdbc.pool.util.MessageFactory; /** - * A simple implementation of a blocking queue with fairness waiting. invocations to method poll(...) will get handed - * out in the order they were received. Locking is fine grained, a shared lock is only used during the first level of - * contention, waiting is done in a lock per thread basis so that order is guaranteed once the thread goes into a - * suspended monitor state. <br> - * Not all of the methods of the {@link java.util.concurrent.BlockingQueue} are implemented. + * A simple implementation of a blocking queue with fairness waiting. Waiters of method poll(...) are handed items in + * the order in which they registered as waiters. Locking is fine grained: a shared lock is only used during the first + * level of contention, while waiting is done on a per-waiter {@link java.util.concurrent.CountDownLatch}, so order is + * guaranteed once a thread has registered itself as a waiter - a caller that has not registered yet may still barge + * ahead when it races for the shared lock. <br> + * Not all of the methods of the {@link java.util.concurrent.BlockingQueue} are supported: the ones that are not throw + * {@link UnsupportedOperationException}. * * @param <E> Type of element in the queue */ @@ -49,16 +51,16 @@ public class FairBlockingQueue<E> implements BlockingQueue<E> { /** * This little sucker is used to reorder the way to do {@link java.util.concurrent.locks.Lock#lock()}, * {@link java.util.concurrent.locks.Lock#unlock()} and {@link java.util.concurrent.CountDownLatch#countDown()} - * during the {@link #poll(long, TimeUnit)} operation. On Linux, it performs much better if we count down while we - * hold the global lock, on Solaris its the other way around. Until we have tested other platforms we only check for - * Linux. + * when {@link #offer(Object)} wakes a waiter whose {@link #poll(long, TimeUnit)} wait the offer completes. On + * Linux, it performs much better if we count down while we hold the global lock, on Solaris its the other way + * around. Until we have tested other platforms we only check for Linux. */ static final boolean isLinux = "Linux".equals(System.getProperty("os.name")) && (!Boolean.getBoolean(FairBlockingQueue.class.getName() + ".ignoreOS")); /** - * Phase one entry lock in order to give out per-thread-locks for the waiting phase we have a phase one lock during - * the contention period. + * Phase one entry lock: held only during the contention period, in order to register the per-waiter latch that + * the waiting phase then parks on. */ final ReentrantLock lock = new ReentrantLock(false); @@ -101,14 +103,11 @@ public class FairBlockingQueue<E> implements BlockingQueue<E> { // item to the waiter it is given to. Like the JDK queue // implementations, reject null elements. Objects.requireNonNull(e); - // During the offer, we will grab the main lock final ReentrantLock lock = this.lock; lock.lock(); ExchangeCountDownLatch<E> c = null; try { - // Check to see if threads are waiting for an object if (!waiters.isEmpty()) { - // If threads are waiting grab the latch for that thread c = waiters.poll(); // Give the object to the thread instead of adding it to the pool c.setItem(e); @@ -130,15 +129,6 @@ public class FairBlockingQueue<E> implements BlockingQueue<E> { return true; } - /** - * Will never timeout, as it invokes the {@link #offer(Object)} method. Once a lock has been acquired, the - * {@inheritDoc} - */ - @Override - public boolean offer(E e, long timeout, TimeUnit unit) throws InterruptedException { - return offer(e); - } - /** * Fair retrieval of an object in the queue. Objects are returned in the order the threads requested them. If the * wait is interrupted but an object was handed over to this waiter all the same, the object is returned and the @@ -154,19 +144,15 @@ public class FairBlockingQueue<E> implements BlockingQueue<E> { // Acquire the global lock until we know what to do lock.lock(); try { - // Check to see if we have objects result = items.poll(); if (result == null && timeout > 0 && !waitersCancelled) { // The queue is empty we will wait for an object ExchangeCountDownLatch<E> c = new ExchangeCountDownLatch<>(1); - // Add to the bottom of the wait list waiters.addLast(c); - // Unlock the global lock lock.unlock(); boolean didtimeout = true; InterruptedException interruptedException = null; try { - // Wait for the specified timeout didtimeout = !c.await(timeout, unit); } catch (InterruptedException ix) { interruptedException = ix; @@ -226,15 +212,12 @@ public class FairBlockingQueue<E> implements BlockingQueue<E> { public Future<E> pollAsync() { Future<E> result = null; final ReentrantLock lock = this.lock; - // Grab the global lock lock.lock(); try { - // Check to see if we have objects in the queue E item = items.poll(); if (item == null) { // Queue is empty, add ourselves as waiters ExchangeCountDownLatch<E> c = new ExchangeCountDownLatch<>(1); - // Return a future that will wait for the object ItemFuture<E> future = new ItemFuture<>(c); // Tell the latch which future its item is meant for, before the // waiter becomes visible to cancelWaiters() @@ -250,7 +233,6 @@ public class FairBlockingQueue<E> implements BlockingQueue<E> { } result = future; } else { - // Return a future with the item result = new ItemFuture<>(item); } } finally { @@ -378,6 +360,11 @@ public class FairBlockingQueue<E> implements BlockingQueue<E> { } } + @Override + public boolean isEmpty() { + return size() == 0; + } + // The queue methods below are not used by the connection pool implementation. @Override @@ -385,6 +372,16 @@ public class FairBlockingQueue<E> implements BlockingQueue<E> { return offer(e); } + /** + * Will never time out, as it invokes the {@link #offer(Object)} method: the element is always inserted, so the + * timeout is never applied and, despite the declaration, {@link InterruptedException} is never thrown. + * {@inheritDoc} + */ + @Override + public boolean offer(E e, long timeout, TimeUnit unit) throws InterruptedException { + return offer(e); + } + /** * {@inheritDoc} * @@ -416,6 +413,10 @@ public class FairBlockingQueue<E> implements BlockingQueue<E> { return Integer.MAX_VALUE - size(); } + /** + * Delegates to {@link #poll(long, TimeUnit)} with an effectively unbounded timeout, so the wait also ends the way + * that method documents for a queue whose waiters {@link #cancelWaiters()} has failed. {@inheritDoc} + */ @Override public E take() throws InterruptedException { return this.poll(Long.MAX_VALUE, TimeUnit.MILLISECONDS); @@ -453,11 +454,6 @@ public class FairBlockingQueue<E> implements BlockingQueue<E> { throw new UnsupportedOperationException(MessageFactory.get("fairBlockingQueue.unsupportedContainsAll")); } - @Override - public boolean isEmpty() { - return size() == 0; - } - /** * {@inheritDoc} * --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
