This is an automated email from the ASF dual-hosted git repository. rmaucher pushed a commit to branch main in repository https://gitbox.apache.org/repos/asf/tomcat.git
commit 04146ef0f5690279bef3541e6e29bfaaf9ed6629 Author: opencode <[email protected]> AuthorDate: Thu Nov 5 17:00:08 2026 +0100 Keep draining tracked statements after one close fails with an Error StatementEntry.close() caught only Exception around isClosed() and close(). A non-fatal Error from a driver or wrapper escaped StatementFinalizer.closeInvoked(), so the remaining entries in the drain were neither closed nor re-added and the session went back to the pool with them open. Catch Throwable in both guards, apply handleThrowable, and treat a non-fatal Error as the failed close that keeps the entry on the retry list. Add a test where the first statement fails with an Error and the next is still closed. --- .../jdbc/pool/interceptor/StatementFinalizer.java | 13 +++++-- .../pool/interceptor/TestStatementFinalizer.java | 40 ++++++++++++++++++++++ 2 files changed, 50 insertions(+), 3 deletions(-) diff --git a/modules/jdbc-pool/src/main/java/org/apache/tomcat/jdbc/pool/interceptor/StatementFinalizer.java b/modules/jdbc-pool/src/main/java/org/apache/tomcat/jdbc/pool/interceptor/StatementFinalizer.java index 2d538aa241..fe7f670586 100644 --- a/modules/jdbc-pool/src/main/java/org/apache/tomcat/jdbc/pool/interceptor/StatementFinalizer.java +++ b/modules/jdbc-pool/src/main/java/org/apache/tomcat/jdbc/pool/interceptor/StatementFinalizer.java @@ -32,6 +32,7 @@ import org.apache.juli.logging.LogFactory; import org.apache.tomcat.jdbc.pool.ConnectionPool; import org.apache.tomcat.jdbc.pool.PoolProperties; import org.apache.tomcat.jdbc.pool.PooledConnection; +import org.apache.tomcat.jdbc.pool.util.ExceptionUtils; import org.apache.tomcat.jdbc.pool.util.MessageFactory; /** @@ -278,10 +279,12 @@ public class StatementFinalizer extends AbstractCreateStatementInterceptor { try { shallClose = !statement.isClosed(); answered = true; - } catch (Exception ignore) { + } catch (Throwable ignore) { // A driver that cannot say whether the statement is closed // must not get to skip the close attempt - the statement - // may still hold server-side state. + // may still hold server-side state. A non-fatal Error is the + // same refused answer as an exception. + ExceptionUtils.handleThrowable(ignore); shallClose = true; answered = false; } @@ -290,7 +293,11 @@ public class StatementFinalizer extends AbstractCreateStatementInterceptor { try { statement.close(); closed = true; - } catch (Exception ignore) { + } catch (Throwable ignore) { + // The failed close is reported as false so closeInvoked + // keeps the entry for the next attempt; an Error must not + // abort the rest of the drain. + ExceptionUtils.handleThrowable(ignore); if (log.isDebugEnabled()) { log.debug("Unable to close statement upon connection close.", ignore); } diff --git a/modules/jdbc-pool/src/test/java/org/apache/tomcat/jdbc/pool/interceptor/TestStatementFinalizer.java b/modules/jdbc-pool/src/test/java/org/apache/tomcat/jdbc/pool/interceptor/TestStatementFinalizer.java index 123414304a..e713de96dc 100644 --- a/modules/jdbc-pool/src/test/java/org/apache/tomcat/jdbc/pool/interceptor/TestStatementFinalizer.java +++ b/modules/jdbc-pool/src/test/java/org/apache/tomcat/jdbc/pool/interceptor/TestStatementFinalizer.java @@ -501,6 +501,46 @@ class TestStatementFinalizer extends JdbcPoolBaseTest { }); } + /* A fake statement whose close fails with a non-fatal Error. */ + private static Statement fakedErrorStatement() { + return (Statement) Proxy.newProxyInstance( + TestStatementFinalizer.class.getClassLoader(), + new Class<?>[] { Statement.class }, + (p, m, a) -> { + switch (m.getName()) { + case "close": + throw new NoClassDefFoundError("simulated stopped context in close()"); + case "isClosed": + return Boolean.FALSE; + case "equals": + return Boolean.valueOf(p == (a == null || a.length == 0 ? null : a[0])); + case "hashCode": + return Integer.valueOf(System.identityHashCode(p)); + default: + return null; + } + }); + } + + @Test + void anErrorFromOneCloseDoesNotAbortTheDrain() throws Exception { + StatementFinalizer finalizer = new StatementFinalizer(); + Method create = Connection.class.getMethod("createStatement"); + finalizer.reset(null, null); + + Statement failing = fakedErrorStatement(); + Statement following = fakedStatement(); + finalizer.createStatement(null, create, null, failing, 0L); + finalizer.createStatement(null, create, null, following, 0L); + + Assertions.assertDoesNotThrow(finalizer::closeInvoked, + "a non-fatal Error from one statement close must not escape the drain"); + Assertions.assertTrue(closeWasAsked(following), + "the statement after the failing one must still be closed"); + Assertions.assertEquals(1, tracked(finalizer), + "the entry whose close failed must stay on the list for the next attempt"); + } + @Test void aCloseTheDriverRefusedStaysOnTheListForANextAttempt() throws Exception { // A return-time close the driver refused must not be dropped: the --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
