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 2eb87ff701146f475d1ede5e1a21fa80b37cfa1d Author: opencode <[email protected]> AuthorDate: Thu Nov 5 16:12:32 2026 +0100 Keep purging cached statements after one close fails with an Error CachedStatement.forceClose() caught only SQLException. A driver close that threw a RuntimeException or non-fatal LinkageError escaped the disconnect purge, so the statements queued after the failing one were not closed. Catch Throwable, apply handleThrowable, log the existing warning and return, so the purge continues. Add a test where one parked statement fails its close with an Error and the next is still closed. --- .../jdbc/pool/interceptor/StatementCache.java | 6 +- .../TestStatementCachePurgeCloseFailure.java | 117 +++++++++++++++++++++ 2 files changed, 122 insertions(+), 1 deletion(-) diff --git a/modules/jdbc-pool/src/main/java/org/apache/tomcat/jdbc/pool/interceptor/StatementCache.java b/modules/jdbc-pool/src/main/java/org/apache/tomcat/jdbc/pool/interceptor/StatementCache.java index f2f6384fb1..8f153ce37c 100644 --- a/modules/jdbc-pool/src/main/java/org/apache/tomcat/jdbc/pool/interceptor/StatementCache.java +++ b/modules/jdbc-pool/src/main/java/org/apache/tomcat/jdbc/pool/interceptor/StatementCache.java @@ -40,6 +40,7 @@ import org.apache.tomcat.jdbc.pool.JdbcInterceptor; import org.apache.tomcat.jdbc.pool.PoolProperties.InterceptorProperty; import org.apache.tomcat.jdbc.pool.PooledConnection; import org.apache.tomcat.jdbc.pool.jmx.JmxUtil; +import org.apache.tomcat.jdbc.pool.util.ExceptionUtils; import org.apache.tomcat.jdbc.pool.util.MessageFactory; /** @@ -792,7 +793,10 @@ public class StatementCache extends StatementDecoratorInterceptor implements Sta removeStatement(this); try { super.closeInvoked(); - } catch (SQLException x) { + } catch (Throwable x) { + // A non-fatal Error from the driver's close is a failed close + // here, not a reason to abort the remainder of the purge. + ExceptionUtils.handleThrowable(x); log.warn(MessageFactory.get("statementCache.unableToCloseDiscardedStatement", getSql()), x); } } diff --git a/modules/jdbc-pool/src/test/java/org/apache/tomcat/jdbc/pool/interceptor/TestStatementCachePurgeCloseFailure.java b/modules/jdbc-pool/src/test/java/org/apache/tomcat/jdbc/pool/interceptor/TestStatementCachePurgeCloseFailure.java new file mode 100644 index 0000000000..9c8516561b --- /dev/null +++ b/modules/jdbc-pool/src/test/java/org/apache/tomcat/jdbc/pool/interceptor/TestStatementCachePurgeCloseFailure.java @@ -0,0 +1,117 @@ +/* + * 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.interceptor; + +import java.lang.reflect.Constructor; +import java.lang.reflect.InvocationTargetException; +import java.lang.reflect.Method; +import java.lang.reflect.Proxy; +import java.sql.Connection; +import java.sql.PreparedStatement; +import java.sql.Statement; +import java.util.concurrent.atomic.AtomicBoolean; + +import org.junit.jupiter.api.Assertions; +import org.junit.jupiter.api.Test; + +import org.apache.tomcat.jdbc.JdbcPoolBaseTest; +import org.apache.tomcat.jdbc.pool.DataSource; +import org.apache.tomcat.jdbc.pool.PooledConnection; + +/** + * A driver statement close that throws an unchecked failure, or a non-fatal + * Error, while the disconnect purge discards cached statements must not abort + * the rest of the purge. Every entry the purge owns is removed and counted + * before the close runs, but the statements still waiting behind the failing + * one must also be closed. + */ +class TestStatementCachePurgeCloseFailure extends JdbcPoolBaseTest { + + /** Arms the next prepared statement to fail its close with an Error. */ + private static final AtomicBoolean FAIL_NEXT_CLOSE = new AtomicBoolean(); + + /** A cache that can arm one prepared statement to fail its close. */ + public static class ErrorCloseStatementCache extends StatementCache { + + @Override + protected Object createDecorator(Object proxy, Method method, Object[] args, + Object statement, Constructor<?> constructor, String sql) + throws InstantiationException, IllegalAccessException, InvocationTargetException { + if (compare(PREPARE_STATEMENT, method.getName()) + && FAIL_NEXT_CLOSE.compareAndSet(true, false)) { + statement = errorClose((PreparedStatement) statement); + } + return super.createDecorator(proxy, method, args, statement, constructor, sql); + } + } + + private static PreparedStatement errorClose(PreparedStatement delegate) { + return (PreparedStatement) Proxy.newProxyInstance( + TestStatementCachePurgeCloseFailure.class.getClassLoader(), + new Class<?>[] { PreparedStatement.class }, + (proxy, method, args) -> { + if ("close".equals(method.getName())) { + throw new NoClassDefFoundError("simulated stopped context in close()"); + } + try { + return args == null ? method.invoke(delegate) : method.invoke(delegate, args); + } catch (InvocationTargetException x) { + throw x.getCause(); + } + }); + } + + @Test + void anErrorFromOneCloseDoesNotAbortThePurge() throws Exception { + DataSource ds = newDataSource(); + ds.getPoolProperties().setJdbcInterceptors( + ErrorCloseStatementCache.class.getName() + "(prepared=true,max=5)"); + ds.getPoolProperties().setMaxActive(1); + ds.getPoolProperties().setTimeBetweenEvictionRunsMillis(60000); + ds.createPool(); + + Connection shared = ds.getConnection(); + try { + ErrorCloseStatementCache cache = + interceptorOf(shared, ErrorCloseStatementCache.class); + PooledConnection pcon = (PooledConnection) shared.unwrap(PooledConnection.class); + + FAIL_NEXT_CLOSE.set(true); + PreparedStatement first = shared.prepareStatement("SELECT 1"); + Statement firstDriver = driverStatement(first); + PreparedStatement second = shared.prepareStatement("SELECT 2"); + Statement secondDriver = driverStatement(second); + first.close(); + second.close(); + Assertions.assertEquals(2, cache.getCacheSize(), + "precondition: both statements must be parked"); + + Assertions.assertDoesNotThrow(() -> cache.disconnected(ds.getPool(), pcon, false), + "the purge must report the failing close, not abort the borrow or close path"); + Assertions.assertEquals(0, cache.getCacheSize(), + "every purged entry must give its counter slot back"); + Assertions.assertEquals(0, cache.getCacheSizePerConnection(), + "the cache of the connection must be empty"); + Assertions.assertTrue(secondDriver.isClosed(), + "a statement after the failing one must still be closed"); + Assertions.assertFalse(firstDriver.isClosed(), + "the statement whose close failed with an Error could not be closed"); + } finally { + shared.close(); + } + } +} --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
