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 0feeef1b2573adc216d90202123e6bc9bb917aa9 Author: opencode <[email protected]> AuthorDate: Thu Oct 29 23:07:16 2026 +0100 FairBlockingQueue: reject null elements to honour the BlockingQueue contract FairBlockingQueue uses absence of an item as its null sentinel internally (poll() gates on items.poll() == null, async delivery on getItem() == null), so an accepted null element made the queue report non-empty while poll() returned null, and an offer(null) racing a registered waiter silently consumed the waiter's slot while delivering nothing. No production path inserts null today, but the class is public and the interface contract requires rejection. offer() now throws NullPointerException for a null element before taking the lock, which covers add(), put(), addAll() and the timed offer() since all of them delegate to offer(). Adds tests pinning rejection on every insertion path and that a rejected offer leaves a registered waiter intact. --- .../apache/tomcat/jdbc/pool/FairBlockingQueue.java | 9 ++++++ .../tomcat/jdbc/pool/TestFairBlockingQueue.java | 35 ++++++++++++++++++++-- 2 files changed, 42 insertions(+), 2 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 6f13346e22..4b23b33ef9 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 @@ -22,6 +22,7 @@ import java.util.Iterator; import java.util.LinkedList; import java.util.List; import java.util.NoSuchElementException; +import java.util.Objects; import java.util.concurrent.BlockingQueue; import java.util.concurrent.CancellationException; import java.util.concurrent.CountDownLatch; @@ -87,9 +88,17 @@ public class FairBlockingQueue<E> implements BlockingQueue<E> { // The queue methods below are used by the connection pool implementation. /** * Will always return true, queue is unbounded. {@inheritDoc} + * + * @throws NullPointerException - if the specified element is null */ @Override public boolean offer(E e) { + // The queue uses null as its absence-of-item sentinel when polling the + // items, so a null element would make a stored item indistinguishable + // from an empty queue and a null hand-off would silently deliver no + // 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(); diff --git a/modules/jdbc-pool/src/test/java/org/apache/tomcat/jdbc/pool/TestFairBlockingQueue.java b/modules/jdbc-pool/src/test/java/org/apache/tomcat/jdbc/pool/TestFairBlockingQueue.java index 0a30d26a11..fc89954c82 100644 --- a/modules/jdbc-pool/src/test/java/org/apache/tomcat/jdbc/pool/TestFairBlockingQueue.java +++ b/modules/jdbc-pool/src/test/java/org/apache/tomcat/jdbc/pool/TestFairBlockingQueue.java @@ -38,8 +38,9 @@ import org.junit.jupiter.api.Test; * longest waiting consumer, deregistration of cancelled asynchronous * polls, the terminal failure of an asynchronous poll whose timed get * times out, the termination of every outstanding poll once the waiters - * are cancelled, and the collection operations the queue deliberately - * leaves unsupported. + * are cancelled, the collection operations the queue deliberately + * leaves unsupported, and the rejection of null elements by every + * insertion path. */ class TestFairBlockingQueue { @@ -93,6 +94,36 @@ class TestFairBlockingQueue { Assertions.assertTrue(seen.contains("added")); } + @Test + void nullElementsAreRejectedByEveryInsertionPath() throws Exception { + FairBlockingQueue<String> queue = new FairBlockingQueue<>(); + Assertions.assertThrows(NullPointerException.class, () -> queue.offer(null)); + Assertions.assertThrows(NullPointerException.class, () -> queue.add(null)); + Assertions.assertThrows(NullPointerException.class, () -> queue.put(null)); + Assertions.assertThrows(NullPointerException.class, + () -> queue.offer(null, 10, TimeUnit.MILLISECONDS)); + Assertions.assertThrows(NullPointerException.class, + () -> queue.addAll(Collections.singletonList(null))); + Assertions.assertEquals(0, queue.size(), "a rejected element must not enter the queue"); + // the lookups that take an Object must keep working on a queue that + // can hold no null element + Assertions.assertFalse(queue.contains(null)); + Assertions.assertFalse(queue.remove(null)); + } + + @Test + void rejectedNullOfferDoesNotConsumeTheWaiterSlot() throws Exception { + FairBlockingQueue<String> queue = new FairBlockingQueue<>(); + Future<String> future = queue.pollAsync(); // registers a waiter + Assertions.assertThrows(NullPointerException.class, () -> queue.offer(null), + "a null hand-off would deliver no item to the registered waiter"); + Assertions.assertFalse(future.isDone(), + "a rejected offer must not hand a phantom element to the waiter"); + Assertions.assertTrue(queue.offer("real")); + Assertions.assertEquals("real", future.get(5, TimeUnit.SECONDS), + "the waiter must still be registered and receive the next element"); + } + @Test void takerBlocksUntilElementArrives() throws Exception { final FairBlockingQueue<String> queue = new FairBlockingQueue<>(); --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
