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 167f481aa3fc285451e3a1e2baece4d4f6ad82aa Author: opencode <[email protected]> AuthorDate: Sun Oct 25 21:28:50 2026 +0100 Report never-connected pooled connections as invalid during validation A wake-up placeholder that no borrower took can linger in the idle queue without an underlying connection. Validating it now reports it as unusable instead of throwing a NullPointerException, so the idle sweep releases it like any other bad connection rather than aborting every cycle and leaving all idle connections untested. --- .../apache/tomcat/jdbc/pool/PooledConnection.java | 9 +- .../jdbc/pool/TestUnconnectedPlaceholder.java | 132 +++++++++++++++++++++ 2 files changed, 140 insertions(+), 1 deletion(-) diff --git a/modules/jdbc-pool/src/main/java/org/apache/tomcat/jdbc/pool/PooledConnection.java b/modules/jdbc-pool/src/main/java/org/apache/tomcat/jdbc/pool/PooledConnection.java index 1bce6d72cd..2bc0b0d848 100644 --- a/modules/jdbc-pool/src/main/java/org/apache/tomcat/jdbc/pool/PooledConnection.java +++ b/modules/jdbc-pool/src/main/java/org/apache/tomcat/jdbc/pool/PooledConnection.java @@ -507,9 +507,16 @@ public class PooledConnection implements PooledConnectionMBean { * {@link PoolConfiguration#setValidationInterval(long)} has been called with a positive value. * <p> * false if the validation failed. The caller should close the connection if false is returned since a session could have been left in - * an unknown state during initialization. + * an unknown state during initialization. A connection that was never connected, or has already been disconnected, is always reported + * as invalid. */ public boolean validate(int validateAction,String sql) { + if (connection == null) { + // never connected - a wake-up placeholder that no borrower took + // - or already disconnected; there is nothing to validate and it + // cannot be handed out + return false; + } if (this.isDiscarded()) { return false; } diff --git a/modules/jdbc-pool/src/test/java/org/apache/tomcat/jdbc/pool/TestUnconnectedPlaceholder.java b/modules/jdbc-pool/src/test/java/org/apache/tomcat/jdbc/pool/TestUnconnectedPlaceholder.java new file mode 100644 index 0000000000..ecfbe650f5 --- /dev/null +++ b/modules/jdbc-pool/src/test/java/org/apache/tomcat/jdbc/pool/TestUnconnectedPlaceholder.java @@ -0,0 +1,132 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.apache.tomcat.jdbc.pool; + +import java.sql.Connection; +import java.util.concurrent.CountDownLatch; +import java.util.concurrent.TimeUnit; +import java.util.concurrent.atomic.AtomicBoolean; + +import static org.junit.jupiter.api.Assertions.assertNotNull; +import static org.junit.jupiter.api.Assertions.assertTrue; +import org.junit.jupiter.api.Test; + +import org.apache.tomcat.jdbc.JdbcPoolBaseTest; + +/** + * A wake-up placeholder that no borrower took stays in the idle queue with + * no underlying connection. Validating it during an idle sweep must report it + * as unusable so it is released like any other bad connection - it must not + * abort the whole sweep cycle, which would leave every idle connection + * untested for as long as the placeholder lingers. + */ +class TestUnconnectedPlaceholder extends JdbcPoolBaseTest { + + /* + * Pool properties that notice - and widen - the window inside + * borrowConnection() after an interrupted queue wait has removed this + * thread's latch from the queue but before the pool wait count is + * decremented. getPropagateInterruptState() is consulted exactly there, + * and (with JMX off and no pool close in flight) nowhere else. + */ + private static final class InterruptWindowProbe extends PoolProperties { + + private static final long serialVersionUID = 1L; + + private final CountDownLatch inWindow = new CountDownLatch(1); + private final AtomicBoolean armed = new AtomicBoolean(); + + @Override + public boolean getPropagateInterruptState() { + if (armed.compareAndSet(true, false)) { + inWindow.countDown(); + sleep(300); + } + return super.getPropagateInterruptState(); + } + + void arm() { + armed.set(true); + } + + boolean awaitInWindow() throws InterruptedException { + return inWindow.await(5, TimeUnit.SECONDS); + } + } + + @Test + void idleSweepRemovesAPlaceholderThatNoBorrowerTook() throws Exception { + InterruptWindowProbe props = new InterruptWindowProbe(); + props.setUrl(getUrl()); + props.setDriverClassName("org.h2.Driver"); + props.setUsername("sa"); + props.setPassword(""); + props.setJmxEnabled(false); + props.setInitialSize(0); + // one above the placeholder count so the age-based eviction guard + // (minIdle < idle) never removes it: only the idle validation can + props.setMinIdle(1); + props.setMaxActive(1); + props.setMaxIdle(1); + // <= 0 means: wait forever for a notification + props.setMaxWait(-1); + props.setTestWhileIdle(true); + props.setTimeBetweenEvictionRunsMillis(200); + DataSource ds = manage(new DataSource()); + ds.setPoolProperties(props); + ds.createPool(); + + Connection holder = ds.getConnection(); // the only slot is taken + PooledConnection pc = + interceptorOf(holder, ProxyConnection.class).getConnection(); + ConnectionPool pool = ds.getPool(); + + final Throwable[] waiterFailure = new Throwable[1]; + Thread waiter = new Thread(() -> { + try { + ds.getConnection(); + } catch (Throwable x) { + waiterFailure[0] = x; + } + }, "interrupted-waiter"); + waiter.start(); + awaitUntil(() -> ds.getWaitCount() == 1); + + props.arm(); + waiter.interrupt(); + assertTrue(props.awaitInWindow(), + "waiter should have reached the window between removing its " + + "queue latch and decrementing the pool wait count"); + + // Destroy the connection while the waiter still counts although its + // latch is gone: the wake-up placeholder is queued into idle and no + // borrower will ever take it. The holder is deliberately left open; + // closing it would return the connection a second time through the + // proxy and muddy what this test observes. + pool.release(pc); + waiter.join(AWAIT_TIMEOUT_MS); + assertNotNull(waiterFailure[0], + "the interrupted waiter must have failed its borrow"); + + // The sweeper runs every 200 ms. Validating the never-connected + // placeholder must not abort the cycle; the placeholder must be + // released like any other unusable idle connection, so both the + // idle count and the pool size drop to zero. + awaitUntil(() -> ds.getIdle() == 0 && ds.getSize() == 0); + awaitSessions(0); + } +} --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
