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 081ba3984aa1cc886ca35b04a2c07af55c3b4608 Author: opencode <[email protected]> AuthorDate: Mon Nov 2 21:28:54 2026 +0100 Make the pool-exhaustion exception type authoritative on the borrow-wait paths PoolExhaustedException is the type the pool throws to say "the pool is empty and no connection could be had", but the paths around it told three different stories: - getConnectionAsync() converted the failed no-wait attempt into an async wait by looking for a "NoWait" marker inside the exception's localized message. Control flow thus depended on the English bundle text: a rewording or translation of the poolEmpty message would turn "start waiting asynchronously" into a thrown exception, a message without the marker was assumed to be a refusal for other reasons, and any unrelated SQLException whose text happened to carry the marker would have been swallowed into a wait. The decision now reads the exception's type. A zero-wait attempt is only ever refused with the no-wait exhaustion variant, so the type carries exactly the information the marker carried, and every other failure of the attempt propagates to the caller untouched. - Both propagateInterruptState property javadocs promised that a thread whose borrow wait is interrupted "will clear the interrupt flag and throw a PoolExhaustedException" while the pool threw a plain SQLException carrying the InterruptedException as its cause. The code is the correct side and the javadoc is corrected to it: an interrupted wait is not pool exhaustion, which the exception's own definition reserves for "the pool is exhausted and no connections are available", so the interrupted wait keeps failing with the plain SQLException and only the two real exhaustion exits keep throwing the named type. TestInterruptedBorrowWait pins what no test pinned before, which is why the divergence survived: the interrupted wait fails with a SQLException caused by the InterruptedException and deliberately not with the exhaustion type; propagateInterruptState governs only whether the thread's interrupt flag stays set, never the failure; and an async borrow on an exhausted pool is absorbed into a wait that the freed connection serves. --- .../apache/tomcat/jdbc/pool/ConnectionPool.java | 20 +-- .../apache/tomcat/jdbc/pool/PoolConfiguration.java | 7 +- .../jdbc/pool/TestInterruptedBorrowWait.java | 165 +++++++++++++++++++++ 3 files changed, 180 insertions(+), 12 deletions(-) diff --git a/modules/jdbc-pool/src/main/java/org/apache/tomcat/jdbc/pool/ConnectionPool.java b/modules/jdbc-pool/src/main/java/org/apache/tomcat/jdbc/pool/ConnectionPool.java index c859e9e317..818bf2cfe2 100644 --- a/modules/jdbc-pool/src/main/java/org/apache/tomcat/jdbc/pool/ConnectionPool.java +++ b/modules/jdbc-pool/src/main/java/org/apache/tomcat/jdbc/pool/ConnectionPool.java @@ -212,15 +212,17 @@ public class ConnectionPool { if (pc != null) { return new ConnectionFuture(pc); } - } catch (SQLException x) { - // JDBC exceptions may carry no message at all; a missing - // "NoWait" marker must be assumed for those, so that the - // original failure reaches the caller instead of a - // NullPointerException from dereferencing the message - String msg = x.getMessage(); - if (msg == null || msg.indexOf("NoWait") < 0) { - throw x; - } + } catch (PoolExhaustedException x) { + // A wait of zero is only refused with the no-wait variant of + // this exception, so the type itself is the "pool is empty + // right now" signal that this path converts into an async + // wait. Discriminating on the type rather than on a marker + // inside the localized message keeps the conversion alive + // when the message is reworded or translated. Every other + // SQLException this attempt can throw - a closed pool, an + // interrupted wait, a validation or connect failure - is a + // failure of the borrow itself and propagates from the + // method to the caller untouched. } // We can only retrieve a future if the underlying queue supports it. if (idle instanceof FairBlockingQueue<?>) { diff --git a/modules/jdbc-pool/src/main/java/org/apache/tomcat/jdbc/pool/PoolConfiguration.java b/modules/jdbc-pool/src/main/java/org/apache/tomcat/jdbc/pool/PoolConfiguration.java index 30db31dfad..93c59d8966 100644 --- a/modules/jdbc-pool/src/main/java/org/apache/tomcat/jdbc/pool/PoolConfiguration.java +++ b/modules/jdbc-pool/src/main/java/org/apache/tomcat/jdbc/pool/PoolConfiguration.java @@ -1036,7 +1036,7 @@ public interface PoolConfiguration { /** * Returns true if the pool is configured to propagate interrupt state of a thread. A thread waiting for a * connection, can have its wait interrupted, and by default will clear the interrupt flag and throw a - * {@link PoolExhaustedException} + * {@link java.sql.SQLException} reporting the interrupted wait, with the {@link InterruptedException} as its cause. * * @return true if the pool is configured to propagate and not clear the thread interrupt state */ @@ -1045,8 +1045,9 @@ public interface PoolConfiguration { /** * Configure the pool to propagate interrupt state for interrupted threads waiting for a connection A thread waiting * for a connection, can have its wait interrupted, and by default will clear the interrupt flag and throw a - * {@link PoolExhaustedException} If set to true, this behavior will change, while the - * {@link PoolExhaustedException} is still thrown, the threads interrupted state is still set. + * {@link java.sql.SQLException} reporting the interrupted wait, with the {@link InterruptedException} as its cause. + * If set to true, this behavior will change, while the {@link java.sql.SQLException} is still thrown, the threads + * interrupted state is still set. * * @param propagateInterruptState - set to true to not clear, but propagate, a threads interrupted state. */ diff --git a/modules/jdbc-pool/src/test/java/org/apache/tomcat/jdbc/pool/TestInterruptedBorrowWait.java b/modules/jdbc-pool/src/test/java/org/apache/tomcat/jdbc/pool/TestInterruptedBorrowWait.java new file mode 100644 index 0000000000..2112ff3b22 --- /dev/null +++ b/modules/jdbc-pool/src/test/java/org/apache/tomcat/jdbc/pool/TestInterruptedBorrowWait.java @@ -0,0 +1,165 @@ +/* + * 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.sql.SQLException; +import java.util.concurrent.Future; +import java.util.concurrent.TimeUnit; + +import org.junit.jupiter.api.Assertions; +import org.junit.jupiter.api.Test; + +import org.apache.tomcat.jdbc.JdbcPoolBaseTest; + +/** + * What an interrupted borrow wait fails with, and what the asynchronous + * entry point treats as its conversion signal. + * + * <p>The property contract used to promise a + * {@link PoolExhaustedException} for an interrupted wait while the pool + * threw a plain {@link SQLException}. The type was made authoritative + * toward the code, not the promise: an interrupted wait is not pool + * exhaustion - the exception's own definition reserves it for "the pool + * is exhausted and no connections are available" - so an interrupted + * wait must fail with a SQLException carrying the InterruptedException + * as its cause, and only the two real exhaustion exits throw the named + * type.</p> + * + * <p>The asynchronous entry point converts a "the pool is empty right + * now" refusal into an async wait. That decision now reads the + * exception's type rather than a marker inside its localized message, so + * a refusal that means "wait" is still recognized as such whatever the + * message says, and any other failure still reaches the caller.</p> + */ +class TestInterruptedBorrowWait extends JdbcPoolBaseTest { + + private static final class BorrowOutcome { + private volatile Throwable failure; + private volatile boolean interruptedAfterFailure; + } + + /** + * A borrow that blocks in the pool's wait (the pool is fully + * borrowed), parked until the pool itself counts it as a waiter, then + * interrupted. + */ + private Thread startInterruptedBorrow(final DataSource ds, + final BorrowOutcome outcome) throws InterruptedException { + Thread borrower = new Thread(() -> { + try { + ds.getConnection(); + outcome.failure = null; // a connection should never arrive here + } catch (Throwable t) { + outcome.failure = t; + outcome.interruptedAfterFailure = + Thread.currentThread().isInterrupted(); + } + }, "interrupted-borrower"); + borrower.setDaemon(true); + borrower.start(); + long deadline = System.currentTimeMillis() + AWAIT_TIMEOUT_MS; + while (ds.getWaitCount() < 1 && System.currentTimeMillis() < deadline) { + Thread.sleep(10); + } + Assertions.assertEquals(1, ds.getWaitCount(), + "the borrow must be registered as a pool waiter before it " + + "is interrupted"); + borrower.interrupt(); + return borrower; + } + + /** + * Pins the contract the property javadoc now states: the interrupted + * wait fails with a SQLException naming the interrupted wait, with + * the InterruptedException as its cause, and it is deliberately not + * the exhaustion type. + */ + private void assertInterruptedWaitFailsAsDocumented( + DataSource ds, boolean propagateInterruptState) + throws Exception { + ds.getPoolProperties().setInitialSize(0); + ds.getPoolProperties().setMaxActive(1); + ds.getPoolProperties().setPropagateInterruptState( + propagateInterruptState); + ds.createPool(); + Connection held = manage(ds).getConnection(); + + BorrowOutcome outcome = new BorrowOutcome(); + Thread borrower = startInterruptedBorrow(ds, outcome); + borrower.join(AWAIT_TIMEOUT_MS); + Assertions.assertFalse(borrower.isAlive(), + "the interrupted borrow must fail, not keep waiting"); + + Throwable failure = outcome.failure; + Assertions.assertNotNull(failure, "the interrupted borrow must fail"); + Assertions.assertInstanceOf(SQLException.class, failure, + "the interrupted borrow must fail with a SQLException"); + Assertions.assertFalse(failure instanceof PoolExhaustedException, + "an interrupted wait is not pool exhaustion and must not " + + "carry the exhaustion type"); + Assertions.assertInstanceOf(InterruptedException.class, + failure.getCause(), + "the interrupted wait must carry the InterruptedException " + + "as its cause"); + + // The property governs only the thread's flag, never the failure. + Assertions.assertEquals(propagateInterruptState, + outcome.interruptedAfterFailure, + "propagateInterruptState=" + propagateInterruptState + + " must decide whether the interrupt flag stays set"); + + held.close(); + } + + @Test + void interruptedWaitFailsWithDocumentedTypeAndClearsTheFlagByDefault() + throws Exception { + assertInterruptedWaitFailsAsDocumented(newDataSource(), false); + } + + @Test + void propagatedInterruptKeepsTheFlagButNotTheFailureType() + throws Exception { + assertInterruptedWaitFailsAsDocumented(newDataSource(), true); + } + + /** + * The async conversion signal is the exception's type: an exhausted + * pool turns the failed no-wait attempt into a waiter, the + * exhaustion never reaches the caller of {@code getConnectionAsync}, + * and the future is served by the connection that frees up while it + * waits. + */ + @Test + void asyncBorrowConvertsTheExhaustionTypeIntoAWait() throws Exception { + DataSource ds = newDataSource(); + ds.getPoolProperties().setInitialSize(0); + ds.getPoolProperties().setMaxActive(1); + ds.createPool(); + Connection held = ds.getConnection(); + + Future<Connection> future = ds.getConnectionAsync(); + + held.close(); + + Connection served = future.get(AWAIT_TIMEOUT_MS, TimeUnit.MILLISECONDS); + Assertions.assertTrue(served.isValid(2), + "the converted wait must hand out a live connection"); + served.close(); + } +} --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
