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 67883282e1f2a363ff88e23206f7d43a455e7aa1 Author: opencode <[email protected]> AuthorDate: Thu Nov 5 14:44:58 2026 +0100 Treat a non-fatal Error as a failed transaction termination PooledConnection.silentlyCommitTransactionIfNeeded and silentlyRollbackTransactionIfNeeded caught only SQLException and RuntimeException. validate() calls the rollback helper from a finally block outside its main Throwable guard, so a LinkageError from getAutoCommit(), commit() or rollback() escaped and stranded the claimed connection between the idle and busy queues; the commit helper had the same gap. Catch Throwable in both helpers, apply handleThrowable, and keep the existing failed validation result and logging. Apply the same treatment to the defensive statement close in validate's catch block. Add an Error mode to the failing-driver test and cover a rollback Error on the borrow path. --- .../apache/tomcat/jdbc/pool/PooledConnection.java | 30 ++++++++++++++++++---- .../jdbc/pool/TestThrowingBuiltInValidation.java | 15 ++++++++++- 2 files changed, 39 insertions(+), 6 deletions(-) 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 d8a4813887..53098a27a3 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 @@ -269,7 +269,11 @@ public class PooledConnection implements PooledConnectionMBean { if (connection != null) { try { this.disconnect(false); - } catch (Exception x) { + } catch (Throwable x) { + // A non-fatal Error from the previous session's teardown is + // a failed disconnect, not a reason to abort the new + // connection attempt. + ExceptionUtils.handleThrowable(x); if (log.isDebugEnabled()) { log.debug("Unable to disconnect previous connection.", x); } @@ -1169,8 +1173,9 @@ public class PooledConnection implements PooledConnectionMBean { if (stmt != null) { try { stmt.close(); - } catch (Exception ignore2) { - /* NOOP */} + } catch (Throwable ignore2) { + ExceptionUtils.handleThrowable(ignore2); + } } } finally { @@ -1201,7 +1206,11 @@ public class PooledConnection implements PooledConnectionMBean { return true; } catch (SQLException e) { log.debug("Failed to commit transaction", e); - } catch (RuntimeException e) { + } catch (Throwable e) { + // A non-fatal Error is the same failed commit an unchecked + // exception is: it must not escape through the caller's finally + // block and strand the connection. + ExceptionUtils.handleThrowable(e); if (getPoolProperties().getLogValidationErrors()) { log.error(MessageFactory.get("pooledConnection.commitFailed"), e); } else { @@ -1225,7 +1234,10 @@ public class PooledConnection implements PooledConnectionMBean { return true; } catch (SQLException e) { log.debug("Failed to rollback transaction", e); - } catch (RuntimeException e) { + } catch (Throwable e) { + // A non-fatal Error is handled exactly like the unchecked + // failure below. + ExceptionUtils.handleThrowable(e); // A failure to end the transaction is a failure whether the // driver reports it checked or unchecked: the caller reads the // false as a failed validation and takes the session out of @@ -1615,6 +1627,14 @@ public class PooledConnection implements PooledConnectionMBean { } } + /** + * Returns a diagnostic rendering of this pooled connection: the driver's own {@link Connection#toString()} when a + * physical session is present, or {@code null}. The driver's text is not masked and may include the URL, the user + * or other session details, so pool warnings that pass a pooled connection should treat it as best-effort + * diagnostic output rather than as a credential-safe view. + * + * @return the diagnostic rendering + */ @Override public String toString() { return "PooledConnection[" + (connection != null ? connection.toString() : "null") + "]"; diff --git a/modules/jdbc-pool/src/test/java/org/apache/tomcat/jdbc/pool/TestThrowingBuiltInValidation.java b/modules/jdbc-pool/src/test/java/org/apache/tomcat/jdbc/pool/TestThrowingBuiltInValidation.java index 734da7cbe3..96fa1eaa1e 100644 --- a/modules/jdbc-pool/src/test/java/org/apache/tomcat/jdbc/pool/TestThrowingBuiltInValidation.java +++ b/modules/jdbc-pool/src/test/java/org/apache/tomcat/jdbc/pool/TestThrowingBuiltInValidation.java @@ -164,6 +164,14 @@ class TestThrowingBuiltInValidation extends JdbcPoolBaseTest { assertFailingBuiltInCheckOnBorrowDoesNotStrand("rollback", true); } + @Test + void errorRollbackFailureOnBorrowDoesNotStrandTheSlot() throws Exception { + // The transaction helper is called from validate()'s finally block, + // outside the main guard; a non-fatal Error from rollback() must be + // read as the failed termination it is. + assertFailingBuiltInCheckOnBorrowDoesNotStrand("rollback=error", true); + } + @Test void uncheckedRollbackFailureAfterValidationQueryDoesNotEscapeTheFinally() throws Exception { @@ -430,11 +438,16 @@ class TestThrowingBuiltInValidation extends JdbcPoolBaseTest { String armed = FAIL_FROM.get(); if (armed != null) { boolean honestFalse = armed.endsWith("=false"); - String name = honestFalse ? armed.substring(0, armed.length() - "=false".length()) : armed; + boolean error = armed.endsWith("=error"); + String suffix = honestFalse ? "=false" : error ? "=error" : ""; + String name = suffix.isEmpty() ? armed : armed.substring(0, armed.length() - suffix.length()); if (method.getName().equals(name)) { if (honestFalse) { return Boolean.FALSE; } + if (error) { + throw new NoClassDefFoundError("boom from " + name + "()"); + } throw new IllegalStateException("boom from " + name + "()"); } } --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
