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 db813f807c07867298da8aa04528de14d868d24b Author: remm <[email protected]> AuthorDate: Wed Oct 28 21:18:32 2026 +0100 Document how a pooled statement reaches the driver's own class The statement handles the pool hands out are proxies implementing the standard java.sql interfaces, and have been for the default configuration ever since the statement facade was introduced. Tracking interceptors put their own standard-interface proxies under the facade, so an application that cast its statement to the driver's class to reach a driver-specific method found nothing at that layer either: in no configuration the pool offers today does the handle answer to the driver's statement class, and the difference between the layers is invisible from the application. Neither the interceptors nor the reference documentation said so, and what an application does have - isWrapperFor and unwrap, delegated honestly by every layer - was left unstated, along with the one thing that follows from it: what unwrap hands back is the driver's own object, so the facade retiring a handle at the end of the borrow does not retire it. Say all of that in the javadoc of the facade and of the statement finalizer and in the reference documentation, with the qualification that a statement a later interceptor recycles for reuse is parked rather than closed at the return. Pin the contract with tests that the handle is a proxy of the standard interface alone, that isWrapperFor answers truly and unwrap arrives at the very driver statement beneath every combination of wrapping interceptors, that a driver-only method then runs on it, that a cached statement reused by a later borrow unwraps to the same driver object, and that the statement finalizer does close what it tracks when the borrow ends. --- modules/jdbc-pool/doc/jdbc-pool.xml | 24 +- .../apache/tomcat/jdbc/pool/StatementFacade.java | 14 + .../jdbc/pool/interceptor/StatementFinalizer.java | 12 + .../jdbc/pool/TestStatementWrapperContract.java | 301 +++++++++++++++++++++ 4 files changed, 348 insertions(+), 3 deletions(-) diff --git a/modules/jdbc-pool/doc/jdbc-pool.xml b/modules/jdbc-pool/doc/jdbc-pool.xml index 6ea0c54211..181167dc6a 100644 --- a/modules/jdbc-pool/doc/jdbc-pool.xml +++ b/modules/jdbc-pool/doc/jdbc-pool.xml @@ -580,9 +580,14 @@ connection the statement was created from rather than the driver's own connection, and a statement stops working, along with the result sets read from it, once that connection - has been returned to the pool. Switching the facade off used - to leave statement and result set handles usable after the - borrow they came from had ended, which allowed a kept handle + has been returned to the pool. The handles of that facade, + and those of the interceptors that wrap statements beneath + it, implement only the standard <code>java.sql</code> + interfaces, so the driver's own classes are reached with + <code>isWrapperFor()</code> and <code>unwrap()</code>. + Switching the facade off used to leave statement and result + set handles usable after the borrow they came from had + ended, which allowed a kept handle to execute on the session the next borrower held; setting this property to <code>false</code> is therefore not supported. The property is still accepted so that existing @@ -659,6 +664,19 @@ <p>Keeps track of all statements created using <code>createStatement</code>, <code>prepareStatement</code> or <code>prepareCall</code> and closes these statements when the connection is returned to the pool. </p> + <p>The tracked statements are handed to the application as proxies + implementing the standard <code>java.sql</code> statement interfaces, as + is the statement facade the pool always installs in front of this + interceptor; the driver's own statement class is therefore not + reachable by casting. <code>isWrapperFor</code> and <code>unwrap</code> + answer through every layer, and <code>unwrap</code> is the supported way + to reach driver-specific statement methods. A statement the application + unwraps during a borrow is still tracked by this interceptor and is + closed, like the others, when the connection is returned to the pool - + unless an interceptor listed after this one recycles the statement + instead of closing it, as a <code>StatementCache</code> beneath this + interceptor does. + </p> <attributes> <attribute name="trace" required="false"> <p>(boolean as String) Enable tracing of unclosed statements. diff --git a/modules/jdbc-pool/src/main/java/org/apache/tomcat/jdbc/pool/StatementFacade.java b/modules/jdbc-pool/src/main/java/org/apache/tomcat/jdbc/pool/StatementFacade.java index 148b483b1b..22f50b2960 100644 --- a/modules/jdbc-pool/src/main/java/org/apache/tomcat/jdbc/pool/StatementFacade.java +++ b/modules/jdbc-pool/src/main/java/org/apache/tomcat/jdbc/pool/StatementFacade.java @@ -33,6 +33,20 @@ import org.apache.tomcat.jdbc.pool.interceptor.AbstractCreateStatementIntercepto /** * Facade that creates statement and result set proxies. + * <p> + * The handles this facade hands out implement only the standard JDBC + * interfaces, as do the proxies of the interceptors that wrap statements + * beneath it, so the driver's own statement and result set classes are never + * visible to the application through a cast. The <code>Wrapper</code> methods + * are delegated to the driver, and <code>unwrap</code> is the supported way + * to reach driver-specific methods through every layer. An unwrapped driver + * object, however, is a driver object rather than a pooled handle: retiring + * these handles at the end of the borrow does not retire what was already + * unwrapped, and only tracking the statements with + * {@link org.apache.tomcat.jdbc.pool.interceptor.StatementFinalizer} (or the + * application closing them) keeps the statements underneath from outliving + * the borrow - unless an interceptor recycles them for the next borrow, as a + * statement cache does. */ public class StatementFacade extends AbstractCreateStatementInterceptor { 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 7d4adf292c..779d13c96e 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 @@ -47,6 +47,18 @@ import org.apache.tomcat.jdbc.pool.PooledConnection; * this one may swallow the statement's <code>close()</code>, e.g. a statement * cache recycling instead of closing; such entries are simply reclaimed at * the connection return, as they were before.) + * <p> + * Tracked statements are handed to the application as proxies implementing + * the standard JDBC statement interfaces, as is the statement facade the + * pool always installs in front of this interceptor and as are the proxies + * of the other statement-wrapping interceptors; the driver's own statement + * class is therefore not reachable by casting. The <code>Wrapper</code> + * methods are delegated to the driver, and <code>unwrap</code> is the + * supported way to reach driver-specific statement methods. A statement the + * application reaches that way is still tracked here and is closed, like any + * other tracked statement, when the connection is returned - unless an + * interceptor listed after this one recycles the statement instead of + * closing it, as a statement cache beneath this interceptor does. * */ public class StatementFinalizer extends AbstractCreateStatementInterceptor { diff --git a/modules/jdbc-pool/src/test/java/org/apache/tomcat/jdbc/pool/TestStatementWrapperContract.java b/modules/jdbc-pool/src/test/java/org/apache/tomcat/jdbc/pool/TestStatementWrapperContract.java new file mode 100644 index 0000000000..d7e2cca535 --- /dev/null +++ b/modules/jdbc-pool/src/test/java/org/apache/tomcat/jdbc/pool/TestStatementWrapperContract.java @@ -0,0 +1,301 @@ +/* + * 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.Proxy; +import java.sql.CallableStatement; +import java.sql.Connection; +import java.sql.PreparedStatement; +import java.sql.ResultSet; +import java.sql.SQLException; +import java.sql.Statement; +import java.util.Arrays; + +import org.junit.jupiter.api.Assertions; +import org.junit.jupiter.api.Test; + +import org.apache.tomcat.jdbc.JdbcPoolBaseTest; +import org.apache.tomcat.jdbc.pool.interceptor.StatementCache; +import org.apache.tomcat.jdbc.pool.interceptor.StatementDecoratorInterceptor; +import org.apache.tomcat.jdbc.pool.interceptor.StatementFinalizer; + +/** + * The {@link java.sql.Wrapper} contract of the statement handles the pool + * hands out - the statement-side counterpart of {@link TestWrapperContract}. + * + * <p>Every statement handle the application receives is a dynamic proxy that + * implements just the standard JDBC interface of its kind: the pool always + * wraps statements in its {@link StatementFacade} (the + * <code>useStatementFacade</code> property that used to switch the facade off + * is deprecated and has no effect), and the proxies of the interceptors that + * wrap statements beneath the facade implement the same standard interfaces. + * The driver's own statement class is therefore not reachable by casting in + * any configuration. What the pool guarantees instead is the honest Wrapper + * answer: {@code isWrapperFor} says true exactly for what is behind the + * handle and false for what is not, and {@code unwrap} - through every layer + * the pool installs - returns the driver's very own statement object, on + * which driver-only methods execute.</p> + * + * <p>What unwrap deliberately does not promise, and what these tests do not + * pin: the unwrapped object is a driver object, not a handle, so the + * retirement of the pool's handles at the end of the borrow does not reach + * what was already unwrapped. That a {@link StatementFinalizer} is what + * closes the statements underneath at the connection return is pinned here, + * because the interceptor documentation promises exactly that.</p> + */ +class TestStatementWrapperContract extends JdbcPoolBaseTest { + + private static final String FINALIZER = + StatementFinalizer.class.getSimpleName(); + private static final String CACHE = + StatementCache.class.getSimpleName() + "(prepared=true,callable=true,max=10)"; + private static final String DECORATOR = + StatementDecoratorInterceptor.class.getSimpleName(); + + /* The handle is the standard-interface proxy, never the driver object. */ + private static void assertHandleIsTheStandardInterfaceProxy(Statement handle, + Class<?> standardInterface, Class<?> driverClass, String what) { + Assertions.assertTrue(Proxy.isProxyClass(handle.getClass()), + what + ": the application handle must be a dynamic proxy, was " + + handle.getClass().getName()); + Assertions.assertTrue(standardInterface.isInstance(handle), + what + ": the handle must implement its standard JDBC interface"); + Assertions.assertTrue(handle instanceof java.sql.Wrapper, + what + ": the handle must be a Wrapper, as java.sql.Statement demands"); + // Pinning the exact interface list: the documented contract is the + // standard interface and nothing else. Should the pool ever decide to + // mirror driver interfaces onto its handles, this is the pin that + // must be lifted deliberately, together with the documentation. + Assertions.assertArrayEquals(new Class<?>[] { standardInterface }, + handle.getClass().getInterfaces(), + what + ": the handle must implement the standard interface only, " + + "found " + Arrays.toString(handle.getClass().getInterfaces())); + Assertions.assertFalse(driverClass.isInstance(handle), + what + ": the driver's statement class must not be visible to instanceof"); + Assertions.assertThrows(ClassCastException.class, () -> driverClass.cast(handle), + what + ": casting the handle to the driver's statement class must fail"); + } + + /* isWrapperFor/unwrap answer honestly through the layers, to the driver object. */ + private static org.h2.jdbc.JdbcStatement assertDriverReachableByUnwrapping( + Statement handle, Class<? extends org.h2.jdbc.JdbcStatement> driverClass, + String config, String kind) throws SQLException { + String what = kind + " statement under " + config; + Assertions.assertTrue(handle.isWrapperFor(driverClass), + what + ": isWrapperFor must answer true for the driver's own class"); + Assertions.assertFalse(handle.isWrapperFor(Runnable.class), + what + ": isWrapperFor must answer false for an interface nothing " + + "behind the handle provides"); + Assertions.assertThrows(SQLException.class, () -> handle.unwrap(Runnable.class), + what + ": unwrap must fail as an SQLException, not hand back a silent null"); + org.h2.jdbc.JdbcStatement driver = handle.unwrap(driverClass); + Assertions.assertSame(driverStatement(handle), driver, + what + ": unwrap must return the very driver object that " + + "unwrap(Statement.class) reports"); + Assertions.assertSame(driver, handle.unwrap(driverClass), + what + ": a repeated unwrap must return the same object, not a new wrapper"); + Assertions.assertTrue(driver.getClass().getName().startsWith("org.h2."), + what + ": unwrap must reach the driver's own object, got " + + driver.getClass().getName()); + // A driver-only method - declared by no java.sql interface - must + // compute on the object reached this way, proving it is live and is + // really the driver's. + Assertions.assertTrue(driver.isSimpleIdentifier("POOL_TEST"), + what + ": the unwrapped driver object must answer driver-only methods"); + Assertions.assertFalse(driver.isSimpleIdentifier("pool test"), + what + ": and answer them like the driver, not like a stub"); + return driver; + } + + @Test + void theApplicationHandleIsAProxyOfTheStandardInterfaceNotTheDriverStatement() + throws Exception { + DataSource ds = newDataSource(); + ds.createPool(); + + try (Connection pooled = ds.getConnection()) { + try (Statement plain = pooled.createStatement()) { + assertHandleIsTheStandardInterfaceProxy(plain, Statement.class, + org.h2.jdbc.JdbcStatement.class, "a plain statement"); + } + try (PreparedStatement prepared = pooled.prepareStatement("SELECT 1")) { + assertHandleIsTheStandardInterfaceProxy(prepared, PreparedStatement.class, + org.h2.jdbc.JdbcPreparedStatement.class, "a prepared statement"); + } + try (CallableStatement call = pooled.prepareCall("CALL NOW()")) { + assertHandleIsTheStandardInterfaceProxy(call, CallableStatement.class, + org.h2.jdbc.JdbcCallableStatement.class, "a callable statement"); + } + } + + // With the finalizer configured, the application still sees only the + // standard-interface handle: the interceptor's own tracking proxy sits + // beneath the facade, out of the application's reach - which is why + // preserving driver interfaces on that inner proxy could not restore + // what casting lost, and why it is not done. + DataSource tracked = newDataSource(); + tracked.getPoolProperties().setJdbcInterceptors(FINALIZER); + tracked.createPool(); + try (Connection pooled = tracked.getConnection(); + PreparedStatement prepared = pooled.prepareStatement("SELECT 1")) { + assertHandleIsTheStandardInterfaceProxy(prepared, PreparedStatement.class, + org.h2.jdbc.JdbcPreparedStatement.class, + "a prepared statement tracked by the finalizer"); + Object trackingProxy = proxiedObject(prepared); + Assertions.assertNotSame(prepared, trackingProxy, + "the tracking proxy must still sit between the facade and the driver"); + Assertions.assertTrue(Proxy.isProxyClass(trackingProxy.getClass()), + "and the layer beneath the facade must be the interceptor's own proxy"); + } + } + + @Test + @SuppressWarnings("deprecation") + void theDeprecatedFacadelessConfigurationCannotMakeTheDriverStatementVisible() + throws Exception { + DataSource ds = newDataSource(); + ds.getPoolProperties().setUseStatementFacade(false); + ds.createPool(); + + try (Connection pooled = ds.getConnection(); + PreparedStatement prepared = pooled.prepareStatement("SELECT 1")) { + // The deprecated value asks for no facade; it no longer removes + // it, so the handle keeps the same shape as everywhere else and + // casting - the only behaviour that configuration ever bought - + // stays unavailable. + assertHandleIsTheStandardInterfaceProxy(prepared, PreparedStatement.class, + org.h2.jdbc.JdbcPreparedStatement.class, + "a prepared statement from the deprecated facadeless configuration"); + Assertions.assertSame(driverStatement(prepared), + prepared.unwrap(org.h2.jdbc.JdbcPreparedStatement.class), + "unwrap must still reach the driver object through the facade"); + } + } + + @Test + void unwrapReachesTheDriversOwnStatementThroughEveryWrappingInterceptor() + throws Exception { + // The third column marks the configurations in which the cache parks + // the statement it hands out directly: a decorator listed beneath + // the cache becomes the cached delegate, and that is the + // statement-cache's own affair, not the Wrapper contract pinned here. + String[][] configurations = { + { "the statement facade alone", null, "false" }, + { "the statement finalizer beneath the facade", FINALIZER, "false" }, + { "the finalizer and the statement cache beneath the facade", + FINALIZER + ";" + CACHE, "true" }, + { "the statement decorator beneath the facade", DECORATOR, "false" }, + { "the finalizer, the cache and the decorator beneath the facade", + FINALIZER + ";" + CACHE + ";" + DECORATOR, "false" }, + }; + for (String[] configuration : configurations) { + DataSource ds = newDataSource(); + if (configuration[1] != null) { + ds.getPoolProperties().setJdbcInterceptors(configuration[1]); + } + ds.createPool(); + + try (Connection pooled = ds.getConnection()) { + try (Statement plain = pooled.createStatement()) { + assertDriverReachableByUnwrapping(plain, + org.h2.jdbc.JdbcStatement.class, + configuration[0], "plain"); + } + PreparedStatement prepared = pooled.prepareStatement("SELECT 1"); + org.h2.jdbc.JdbcPreparedStatement preparedDriver = + (org.h2.jdbc.JdbcPreparedStatement) assertDriverReachableByUnwrapping( + prepared, org.h2.jdbc.JdbcPreparedStatement.class, + configuration[0], "prepared"); + // The object reached by unwrapping is usable: the prepared + // query executes through it while the borrow is live. + try (ResultSet rs = preparedDriver.executeQuery()) { + Assertions.assertTrue(rs.next(), + "prepared statement under " + configuration[0] + + ": the unwrapped driver statement must still execute"); + } + prepared.close(); + if ("true".equals(configuration[2])) { + // A second prepare of the same SQL is answered from the + // cache, under fresh handles of every layer: unwrap must + // still report the same driver object beneath them. + try (PreparedStatement reused = pooled.prepareStatement("SELECT 1")) { + Assertions.assertNotSame(prepared, reused, + "the cache hands the parked statement out under a fresh handle"); + Assertions.assertSame(preparedDriver, driverStatement(reused), + "a reused cached statement must unwrap to the same " + + "driver object"); + } + } + try (CallableStatement call = pooled.prepareCall("CALL NOW()")) { + assertDriverReachableByUnwrapping(call, + org.h2.jdbc.JdbcCallableStatement.class, + configuration[0], "callable"); + } + } + } + } + + /** + * The documentation of {@link StatementFinalizer} promises that a + * statement the application unwrapped during the borrow is still tracked: + * unlike the bare facade, which retires only its own handles and leaves + * the driver statement behind to serve whoever holds the physical session + * next, the finalizer closes the statements underneath when the + * connection is returned. + */ + @Test + void theFinalizerClosesTheDriverStatementBeneathTheHandleWhenTheBorrowEnds() + throws Exception { + DataSource ds = newDataSource(); + ds.getPoolProperties().setJdbcInterceptors(FINALIZER); + // force the next borrow onto the same physical session + ds.getPoolProperties().setMaxActive(1); + ds.getPoolProperties().setMaxIdle(1); + ds.createPool(); + + Connection first = ds.getConnection(); + setSessionMarker(first, 21); + PreparedStatement prepared = first.prepareStatement("SELECT 1"); + Statement plain = first.createStatement(); + org.h2.jdbc.JdbcPreparedStatement preparedDriver = + prepared.unwrap(org.h2.jdbc.JdbcPreparedStatement.class); + org.h2.jdbc.JdbcStatement plainDriver = plain.unwrap(org.h2.jdbc.JdbcStatement.class); + Assertions.assertFalse(preparedDriver.isClosed(), + "precondition: the driver statement is open during the borrow"); + first.close(); + + Assertions.assertTrue(preparedDriver.isClosed(), + "the finalizer must close the tracked prepared statement at the return"); + Assertions.assertTrue(plainDriver.isClosed(), + "the finalizer must close the tracked plain statement at the return"); + Assertions.assertThrows(SQLException.class, preparedDriver::execute, + "the unwrapped prepared statement must not run after the borrow ended"); + Assertions.assertThrows(SQLException.class, + () -> plainDriver.execute("SET @POOL_TEST_MARKER = 999"), + "the unwrapped plain statement must not run after the borrow ended"); + + Connection second = ds.getConnection(); + Assertions.assertEquals(21, sessionMarker(second).intValue(), + "the next borrower's session must be untouched"); + try (Statement st = second.createStatement(); + ResultSet rs = st.executeQuery("SELECT 1")) { + Assertions.assertTrue(rs.next(), + "the current borrower must get working handles"); + } + second.close(); + } +} --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
