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 09e189029532ad609af3cd63cc2a45acaa534556 Author: opencode <[email protected]> AuthorDate: Sun Oct 25 18:22:13 2026 +0100 Do not busy-loop when closing the connection pool from an interrupted thread close(force) drains the idle and busy queues with timed polls. When the calling thread carried an interrupt status - a common situation for container stop threads - and propagateInterruptState was true, the InterruptedException handler restored the status inside the drain loop. Polls on the busy queue, a LinkedBlockingQueue, check the status before inspecting their elements and threw again immediately, so no connection was ever removed and close() spun in a hot loop forever, leaving every physical connection open. Clear the status so the drain can progress, remember that an interruption occurred, and restore the status after the drain completes when propagation is requested, preserving the documented semantics of the property. --- .../apache/tomcat/jdbc/pool/ConnectionPool.java | 15 ++- .../tomcat/jdbc/pool/TestInterruptedPoolClose.java | 121 +++++++++++++++++++++ webapps/docs/changelog.xml | 8 ++ 3 files changed, 141 insertions(+), 3 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 abdded19d8..7170a311da 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 @@ -422,6 +422,7 @@ public class ConnectionPool { /* release all idle connections */ BlockingQueue<PooledConnection> pool = (!idle.isEmpty())?idle:(force?busy:idle); + boolean interrupted = false; while (!pool.isEmpty()) { try { //retrieve the next connection @@ -441,14 +442,22 @@ public class ConnectionPool { } } //while } catch (InterruptedException ex) { - if (getPoolProperties().getPropagateInterruptState()) { - Thread.currentThread().interrupt(); - } + // Do not restore the interrupt status here. A poll on a queue + // that checks the status before inspecting its elements would + // then throw again immediately, removing no connection, and + // the loop would spin without progress. Clear the status so + // the polls keep working and remember that we were interrupted + // so the status can be propagated once the drain completes. + Thread.interrupted(); + interrupted = true; } if (pool.isEmpty() && force && pool!=busy) { pool = busy; } } + if (interrupted && getPoolProperties().getPropagateInterruptState()) { + Thread.currentThread().interrupt(); + } if (this.getPoolProperties().isJmxEnabled()) { this.jmxPool = null; } diff --git a/modules/jdbc-pool/src/test/java/org/apache/tomcat/jdbc/pool/TestInterruptedPoolClose.java b/modules/jdbc-pool/src/test/java/org/apache/tomcat/jdbc/pool/TestInterruptedPoolClose.java new file mode 100644 index 0000000000..f2fab578e1 --- /dev/null +++ b/modules/jdbc-pool/src/test/java/org/apache/tomcat/jdbc/pool/TestInterruptedPoolClose.java @@ -0,0 +1,121 @@ +/* + * 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.ArrayList; +import java.util.List; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertTrue; +import org.junit.jupiter.api.Test; + +import org.apache.tomcat.jdbc.JdbcPoolBaseTest; + +/** + * Pool shutdown routinely runs on container-stop threads that already + * carry an interrupt status. The drain in {@code close()} must then still + * close every physical connection rather than spin on polls that throw + * immediately because the interrupt status is pending. + */ +class TestInterruptedPoolClose extends JdbcPoolBaseTest { + + private static final int CONNECTIONS = 3; + + /* Captures the interrupt status observed by the thread that closed. */ + private static final class CloseOutcome { + private volatile boolean interruptedAfterClose; + } + + private CloseOutcome closeFromInterruptedThread(DataSource ds) throws InterruptedException { + CloseOutcome outcome = new CloseOutcome(); + Thread closer = new Thread(() -> { + Thread.currentThread().interrupt(); // as a stop thread may carry + ds.close(true); + outcome.interruptedAfterClose = Thread.currentThread().isInterrupted(); + }, "interrupted-closer"); + closer.setDaemon(true); + closer.start(); + closer.join(AWAIT_TIMEOUT_MS); + assertFalse(closer.isAlive(), "close() must complete on an interrupted thread"); + return outcome; + } + + @Test + void interruptedForceCloseClosesBorrowedConnections() throws Exception { + DataSource ds = newDataSource(); + ds.getPoolProperties().setInitialSize(0); + ds.getPoolProperties().setPropagateInterruptState(true); + ds.createPool(); + for (int i = 0; i < CONNECTIONS; i++) { + ds.getConnection(); // stay borrowed - these sit in the busy queue + } + awaitSessions(CONNECTIONS); + + CloseOutcome outcome = closeFromInterruptedThread(ds); + + assertTrue(outcome.interruptedAfterClose, + "propagateInterruptState=true must keep the interrupt status across close()"); + assertEquals(0, ds.getSize()); + assertEquals(0, poolSessions(), + "a forced close from an interrupted thread must close borrowed connections"); + } + + @Test + void interruptedForceCloseClosesIdleAndBorrowedConnections() throws Exception { + DataSource ds = newDataSource(); + ds.getPoolProperties().setInitialSize(0); + ds.getPoolProperties().setPropagateInterruptState(true); + ds.createPool(); + List<Connection> idleOnes = new ArrayList<>(); + for (int i = 0; i < CONNECTIONS * 2; i++) { + idleOnes.add(ds.getConnection()); + } + for (int i = 0; i < CONNECTIONS; i++) { + idleOnes.get(i).close(); // returned - these sit in the idle queue + } + awaitSessions(CONNECTIONS * 2); + + // the drain starts on the idle queue and must then switch to busy + CloseOutcome outcome = closeFromInterruptedThread(ds); + + assertTrue(outcome.interruptedAfterClose, + "propagateInterruptState=true must keep the interrupt status across close()"); + assertEquals(0, ds.getSize()); + assertEquals(0, poolSessions(), + "a forced close must drain both queues despite the interrupt"); + } + + @Test + void interruptedCloseWithoutPropagationClearsInterruptStatus() throws Exception { + DataSource ds = newDataSource(); + ds.getPoolProperties().setInitialSize(0); + ds.createPool(); // propagateInterruptState is false by default + for (int i = 0; i < CONNECTIONS; i++) { + ds.getConnection(); + } + awaitSessions(CONNECTIONS); + + CloseOutcome outcome = closeFromInterruptedThread(ds); + + assertFalse(outcome.interruptedAfterClose, + "propagateInterruptState=false must not leave the interrupt status set"); + assertEquals(0, poolSessions(), + "the drain must proceed after a consumed interrupt"); + } +} diff --git a/webapps/docs/changelog.xml b/webapps/docs/changelog.xml index a556a71ee2..87d967ce64 100644 --- a/webapps/docs/changelog.xml +++ b/webapps/docs/changelog.xml @@ -724,6 +724,14 @@ statements as not poolable. Pull request <pr>1083</pr> provided by Raphael Vullriede. (markt) </fix> + <fix> + Correct a defect that meant closing the connection pool from a thread + that carried an interrupt status, when + <code>propagateInterruptState</code> was set to + <code>true</code>, looped forever without closing any of the pool's + connections. The shutdown drain now always completes and the + interrupted status is propagated once it has. (remm) + </fix> </changelog> </subsection> <subsection name="Other"> --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
