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 7c6bed669bd0844456afc40908d7f82d5012dacd Author: opencode <[email protected]> AuthorDate: Thu Oct 29 09:57:06 2026 +0100 Send the JDBC 4.3 request boundary hints around a pooled borrow The pool never told the driver where a unit of work began or ended on a pooled connection, so a driver that optimises session handling around a request had nothing to optimise. beginRequest() is now sent on the physical connection as the pool hands it to the application, and endRequest() as the application returns it to the pool; a connection discarded rather than returned is not hinted, closing it subsumes the hint. The two methods are looked up reflectively once at class load, as this module still compiles against Java 8, which does not declare them; on a runtime that predates JDBC 4.3 the lookup yields null and the hints are skipped. A hint is advisory, so a driver that refuses one has the refusal logged rather than the borrow or the return around it failed. Patch by: Fernanda Meheust Pull request: https://github.com/apache/tomcat/pull/677 --- .../apache/tomcat/jdbc/pool/ConnectionPool.java | 74 +++++- .../tomcat/jdbc/pool/TestRequestBoundaries.java | 248 +++++++++++++++++++++ webapps/docs/changelog.xml | 11 + 3 files changed, 332 insertions(+), 1 deletion(-) 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 075ffbf414..0705f1e35f 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 @@ -19,6 +19,8 @@ package org.apache.tomcat.jdbc.pool; import java.lang.ref.WeakReference; import java.lang.reflect.Constructor; import java.lang.reflect.InvocationHandler; +import java.lang.reflect.InvocationTargetException; +import java.lang.reflect.Method; import java.lang.reflect.Proxy; import java.sql.Connection; import java.sql.SQLException; @@ -69,6 +71,20 @@ public class ConnectionPool { */ private static final Log log = LogFactory.getLog(ConnectionPool.class); + /** + * The JDBC 4.3 <code>beginRequest()</code> hint, looked up by reflection + * because this module compiles against Java 8, whose + * {@link java.sql.Connection} does not declare it; on a runtime that + * predates JDBC 4.3 the lookup yields null and the hint is not sent. + */ + private static final Method BEGIN_REQUEST = getRequestHintMethod("beginRequest"); + + /** + * The counterpart of {@link #BEGIN_REQUEST}, sent when the application + * returns the connection to the pool. + */ + private static final Method END_REQUEST = getRequestHintMethod("endRequest"); + //=============================================================================== // INSTANCE/QUICK ACCESS VARIABLE //=============================================================================== @@ -378,8 +394,16 @@ public class ConnectionPool { // for the next borrower. (The useDisposableConnectionFacade // property that used to switch this off is deprecated and no // longer has any effect.) - return (Connection)proxyConstructor.newInstance( + Connection connection = (Connection)proxyConstructor.newInstance( new Object[] { new DisposableConnectionFacade(handler) }); + //The handle exists and the borrow is about to start: send the + //JDBC 4.3 hint that opens the request, so a driver that + //optimises per-request session handling knows the unit of work + //begins here. The matching endRequest() is sent at the return, + //in returnConnection(). + invokeRequestHint(BEGIN_REQUEST, con.getConnection()); + //return the connection + return connection; }catch (Exception x) { SQLException s = new SQLException(); s.initCause(x); @@ -407,6 +431,48 @@ public class ConnectionPool { } } + /** + * Looks up one of the JDBC 4.3 request boundary hint methods on + * {@link java.sql.Connection} by name, as they are absent from the + * Java 8 API this module compiles against. + * @param methodName the name of the hint method + * @return the method, or <code>null</code> on a runtime that predates + * JDBC 4.3 + */ + private static Method getRequestHintMethod(String methodName) { + try { + return Connection.class.getMethod(methodName); + } catch (NoSuchMethodException x) { + //Java 8 and earlier: there are no request boundary hints to send + return null; + } + } + + /** + * Sends one of the JDBC 4.3 request boundary hints to a physical + * connection. The hint is advisory - the specification lets the driver + * treat it as a no-op - so a driver that refuses one does not fail the + * borrow or the return around it; the refusal is logged with the + * driver's own cause. + * @param hint one of the hint methods, or <code>null</code> when + * running where they do not exist + * @param connection the physical connection to hint + */ + private static void invokeRequestHint(Method hint, Connection connection) { + if (hint == null) { + return; + } + try { + hint.invoke(connection); + } catch (InvocationTargetException x) { + log.warn("The request boundary hint [" + hint.getName() + + "] was refused by the driver", x.getTargetException()); + } catch (ReflectiveOperationException | IllegalArgumentException x) { + log.warn("Unable to send the request boundary hint [" + + hint.getName() + "]", x); + } + } + /** * Creates and caches a {@link java.lang.reflect.Constructor} used to * instantiate the proxy object of the requested kind (with or without @@ -1195,6 +1261,12 @@ public class ConnectionPool { con.clearWarnings(); con.setStackTrace(null); con.setTimestamp(System.currentTimeMillis()); + //The application is done with the connection: close + //the JDBC 4.3 request that beginRequest() opened in + //setupConnection(). A connection discarded instead + //of going back to the pool is not hinted; it is + //about to be closed, which subsumes the hint. + invokeRequestHint(END_REQUEST, con.getConnection()); if (((idle.size()>=poolProperties.getMaxIdle()) && !poolProperties.isPoolSweeperEnabled()) || (!idle.offer(con))) { if (log.isDebugEnabled()) { log.debug("Connection ["+con+"] will be closed and not returned to the pool, idle["+idle.size()+"]>=maxIdle["+poolProperties.getMaxIdle()+"] idle.offer failed."); diff --git a/modules/jdbc-pool/src/test/java/org/apache/tomcat/jdbc/pool/TestRequestBoundaries.java b/modules/jdbc-pool/src/test/java/org/apache/tomcat/jdbc/pool/TestRequestBoundaries.java new file mode 100644 index 0000000000..bb0491867e --- /dev/null +++ b/modules/jdbc-pool/src/test/java/org/apache/tomcat/jdbc/pool/TestRequestBoundaries.java @@ -0,0 +1,248 @@ +/* + * 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.lang.reflect.InvocationHandler; +import java.lang.reflect.InvocationTargetException; +import java.lang.reflect.Method; +import java.lang.reflect.Proxy; +import java.sql.Connection; +import java.sql.Driver; +import java.sql.DriverPropertyInfo; +import java.sql.SQLException; +import java.sql.SQLFeatureNotSupportedException; +import java.util.Properties; +import java.util.concurrent.atomic.AtomicInteger; +import java.util.logging.Logger; + +import org.junit.jupiter.api.Assertions; +import org.junit.jupiter.api.Assumptions; +import org.junit.jupiter.api.Test; + +import org.apache.tomcat.jdbc.JdbcPoolBaseTest; + +/** + * The JDBC 4.3 request boundary hints: the pool is expected to call + * {@code Connection.beginRequest()} on the physical connection as it hands + * that connection to the application and {@code Connection.endRequest()} + * as the application returns it, so a driver that optimises session + * handling around a unit of work can do so. A hint is advisory, so a + * driver that refuses one must not fail the borrow or the return around + * it, and the pool still has to hand out a working, reusable connection. + * + * <p>Scenario harvested from the contribution of pull request 677, which + * counted the hints on a stub driver; here the counting proxy sits on the + * H2 connection itself, so the assertions also prove the hints travelled + * on the physical session and bracketed each borrow.</p> + */ +class TestRequestBoundaries extends JdbcPoolBaseTest { + + private static final AtomicInteger BEGIN_REQUEST_COUNT = new AtomicInteger(); + private static final AtomicInteger END_REQUEST_COUNT = new AtomicInteger(); + + /* + * Same lookup the pool itself performs: on a Java 8 runtime the hints + * do not exist, the pool sends none, and there is nothing for this + * class to observe. + */ + private static void assumeRequestHintsExist() { + boolean exists; + try { + Connection.class.getMethod("beginRequest"); + exists = true; + } catch (NoSuchMethodException e) { + exists = false; + } + Assumptions.assumeTrue(exists, + "java.sql.Connection has no request boundary hints before Java 9"); + } + + /* + * The hints must be counted where the pool sends them: at the physical + * connection. A driver configured by class name wraps whatever the H2 + * driver returns in a proxy that counts the two hint methods before + * delegating them to H2, and the pool is configured to use that driver + * against the real database. + */ + private DataSource hintCountingDataSource(Class<? extends Driver> driverClass) + throws SQLException { + BEGIN_REQUEST_COUNT.set(0); + END_REQUEST_COUNT.set(0); + DataSource ds = newDataSource(); + ds.setDriverClassName(driverClass.getName()); + // start empty: no physical connection, so no pool-start traffic to + // discount from the counts asserted below + ds.getPoolProperties().setInitialSize(0); + ds.createPool(); + return ds; + } + + @Test + void hintsBracketEachBorrowOfTheSameSession() throws Exception { + assumeRequestHintsExist(); + DataSource ds = hintCountingDataSource(HintCountingDriver.class); + + Assertions.assertEquals(0, BEGIN_REQUEST_COUNT.get(), "no borrow yet"); + Assertions.assertEquals(0, END_REQUEST_COUNT.get(), "no borrow yet"); + + Connection first = ds.getConnection(); + Assertions.assertEquals(1, BEGIN_REQUEST_COUNT.get(), + "beginRequest() must reach the driver as the connection is handed out"); + Assertions.assertEquals(0, END_REQUEST_COUNT.get(), + "endRequest() belongs to the return, not the borrow"); + + first.close(); + Assertions.assertEquals(1, END_REQUEST_COUNT.get(), + "endRequest() must reach the driver as the connection is returned"); + + // a second borrow on the same physical session hints again - the + // hints bracket every unit of work, not just the first one + Connection second = ds.getConnection(); + Assertions.assertEquals(2, BEGIN_REQUEST_COUNT.get()); + Assertions.assertEquals(1, END_REQUEST_COUNT.get()); + second.close(); + Assertions.assertEquals(2, END_REQUEST_COUNT.get()); + + Assertions.assertEquals(1, poolSessions(), + "both borrows ran on the same physical session"); + } + + @Test + void aDriverThatRefusesTheHintsNeitherFailsTheBorrowNorTheReturn() throws Exception { + assumeRequestHintsExist(); + DataSource ds = hintCountingDataSource(FailingHintDriver.class); + + try (Connection con = ds.getConnection()) { + Assertions.assertEquals(1, BEGIN_REQUEST_COUNT.get(), + "the refused hint must still have been attempted"); + // the connection itself is unaffected by the refusal + con.createStatement().close(); + } + Assertions.assertEquals(1, END_REQUEST_COUNT.get(), + "the refused hint must not strand the return of the connection"); + + // the connection stayed poolable across the refused hints + try (Connection con = ds.getConnection()) { + Assertions.assertEquals(2, BEGIN_REQUEST_COUNT.get()); + } + Assertions.assertEquals(1, poolSessions(), + "the refused hints must not have cost the pool its session"); + } + + /** + * Driver that counts the request boundary hints reaching the physical + * connection it returns. Configured through {@code driverClassName}, + * so it must stay a public static class with a no-arg constructor. + */ + public static class HintCountingDriver implements Driver { + + private final Driver delegate = new org.h2.Driver(); + + @Override + public Connection connect(String url, Properties info) throws SQLException { + Connection connection = delegate.connect(url, info); + if (connection == null) { + return null; + } + return (Connection) Proxy.newProxyInstance( + HintCountingDriver.class.getClassLoader(), + new Class<?>[] { Connection.class }, + hintCounter(connection, refuseHints())); + } + + /* + * Overridden by the subclass below; a driver instantiated by class + * name can only be told apart by its own behaviour. + */ + protected boolean refuseHints() { + return false; + } + + @Override + public boolean acceptsURL(String url) throws SQLException { + return delegate.acceptsURL(url); + } + + @Override + public DriverPropertyInfo[] getPropertyInfo(String url, Properties info) throws SQLException { + return delegate.getPropertyInfo(url, info); + } + + @Override + public int getMajorVersion() { + return delegate.getMajorVersion(); + } + + @Override + public int getMinorVersion() { + return delegate.getMinorVersion(); + } + + @Override + public boolean jdbcCompliant() { + return delegate.jdbcCompliant(); + } + + @Override + public Logger getParentLogger() throws SQLFeatureNotSupportedException { + return delegate.getParentLogger(); + } + } + + /** + * Driver that additionally refuses both hints, to pin that the pool + * treats a refused hint as the advisory failure it is. + */ + public static class FailingHintDriver extends HintCountingDriver { + @Override + protected boolean refuseHints() { + return true; + } + } + + /* + * Counts the two hint methods by name and delegates everything, the + * hints included, to the H2 connection; by name, because this suite + * still compiles against Java 8. The refusing configuration makes the + * two hints throw instead of delegating, like a driver that declines + * to take them. + */ + private static InvocationHandler hintCounter(Connection vendor, boolean refuseHints) { + return new InvocationHandler() { + @Override + public Object invoke(Object proxy, Method method, Object[] args) throws Throwable { + String name = method.getName(); + if ("beginRequest".equals(name)) { + BEGIN_REQUEST_COUNT.incrementAndGet(); + if (refuseHints) { + throw new SQLException("beginRequest() refused by the test driver"); + } + } else if ("endRequest".equals(name)) { + END_REQUEST_COUNT.incrementAndGet(); + if (refuseHints) { + throw new SQLException("endRequest() refused by the test driver"); + } + } + try { + return method.invoke(vendor, args); + } catch (InvocationTargetException x) { + throw x.getCause(); + } + } + }; + } +} diff --git a/webapps/docs/changelog.xml b/webapps/docs/changelog.xml index 1c23c4e4d5..527d33083c 100644 --- a/webapps/docs/changelog.xml +++ b/webapps/docs/changelog.xml @@ -1312,6 +1312,17 @@ effect, but it is still accepted so existing configurations keep loading. (remm) </fix> + <add> + Send the JDBC 4.3 request boundary hints on the pooled connections: + <code>beginRequest()</code> is now called on the physical connection + when the pool hands it to the application and + <code>endRequest()</code> when the application returns it, so that a + driver which optimises session handling around a unit of work can do + so. The hints are looked up reflectively and skipped on a Java runtime + that predates JDBC 4.3, and a driver that refuses a hint has the + refusal logged rather than the borrow or the return around it failed. + Pull request <pr>677</pr> provided by Fernanda Meheust. (remm) + </add> </changelog> </subsection> <subsection name="Other"> --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
